Antalya 26.3 allow non matching schema export partition by name - #2220
Antalya 26.3 allow non matching schema export partition by name#2220k-morozov wants to merge 5 commits into
Conversation
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
k-morozov
changed the title
k-morozov
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| - In `strict` mode, a different number of source and destination columns throws `NUMBER_OF_COLUMNS_DOESNT_MATCH`. | ||
| - In `ignore_extra_source_columns_by_position` mode, the destination having more columns than the source throws `NUMBER_OF_COLUMNS_DOESNT_MATCH`. | ||
| - In `ignore_extra_source_columns_by_name` mode, when the source has more columns than the destination, a destination column absent from the source (including a renamed one) throws `THERE_IS_NO_COLUMN`; the mode does not fall back to positional matching in that case. | ||
| - In `ignore_extra_source_columns_by_name` mode, when the source does not have more columns than the destination, a column-count mismatch throws `NUMBER_OF_COLUMNS_DOESNT_MATCH`, same as `strict`. |
There was a problem hiding this comment.
Doesn't make sense to allow equal number of columns with ability to reorder? Now it I want to reorder I can add dummy unused column to source table. It's weird.
It's topic to discussion, not to implement in current PR.
There was a problem hiding this comment.
May be in future we add modes to skip some destination columns.
There was a problem hiding this comment.
Doesn't make sense to allow equal number of columns with ability to reorder?
I also agree with you. I think we should re-design this to something like #2220 (comment)
There was a problem hiding this comment.
@ianton-ru I've been thinking about this. First, I added ignore_extra_source_columns_by_name, which also worked for cases where the number of columns matches. But I think this is the wrong semantics - because the setting's name talks about extra columns, and the user expects the setting to work only for cases with extra columns. If it also overrides matching when the number of columns is the same, that would be misleading. So in one of the last commits I dropped this idea - the setting should be clear and intuitive in its behavior. On the other hand, I now dropped matching by name when the number of columns is the same. I think this is useful functionality, but then the settings need to be made differently. Export partition is an experimental feature and we can change them.
My first idea was to extend export_merge_tree_part_schema_mismatch_mode:
strict_py_pos
strict_by_name
ignore_extra_source_columns_by_position
ignore_extra_source_columns_by_name
So the first 2 modes handle the semantics when the number of columns matches, and the last 2 when it doesn't match.
But this setting has downsides - it's very manual, meaning you can't set it in the system as a default for all exports. Then I thought it would make sense to split it into 2 settings: one for when the number of columns matches, and one for when there are extra columns:
export_partition_matching_columns_policy:
position
name
export_partition_matching_extra_columns_policy:
position
name
This gives flexibility - we can do matching differently in these 2 cases, but this is also a source of problems for the user, since they might forget about the 2 settings. And that worried me.
@arthurpassos , below, suggested a 3rd approach: instead of export_partition_matching_extra_columns_policy, add a flag for whether extra columns are allowed at all or not. And I think this is the most user-friendly option.
Let me know what you think - I think the current PR is a good place to use the settings described above instead of the current ones.
|
Hm... I am not so sure this is the path we should take. It seems like we need two settings instead of one. It has been a few weeks we discussed this and I was on PTO, but I thought we wanted to implement something like: column_match_mode = ['position', 'name'] column_count_mismatch = true/false The current approach seems to fail when the schemas have the same amount of columns but in different order. For example: What do you think? |
4 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Extend the
export_merge_tree_part_schema_mismatch_modesetting byignore_extra_source_columns_by_name, EXPORT PART/EXPORT PARTITION allows a source table with extra trailing columns — they are simply ignored instead of failing withNUMBER_OF_COLUMNS_DOESNT_MATCHwith matching by name.Documentation entry for user-facing changes
...
CI/CD Options
Exclude tests:
Regression jobs to run: