Allow non matching schemas on EXPORT PARTITION#2111
Conversation
0d6a23a to
267e79f
Compare
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>
Signed-off-by: Konstantin Morozov <just.morozov.k@gmail.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 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". |
|
All current CI failures on this PR are pre-existing / unrelated to this PR's changes:
No failure traces back to the changes in this PR. |
| enum class MergeTreePartExportSchemaMismatchMode : uint8_t | ||
| { | ||
| strict, | ||
| ignore_extra_source_columns_by_position, |
There was a problem hiding this comment.
I think name is confusing.
I expect that I can say "ignore columns at positions 3, 5 and 7", but actually it ignores last columns.
May be something like ignore_trailing_extra_columns?
/I'm not good in naming anyway/
There was a problem hiding this comment.
I thought to use 2 modes: ignore_extra_source_columns_by_position and ignore_extra_source_columns_by_name (in the future). Thank you for the example — I agree that the current naming is really confusing.
In the current name, the _by_position suffix meant that all columns from the prefix are matched 1-to-1, and the rest are ignored. I like the name ignore_trailing_extra_columns as the base. I'll suggest adding information that this is about the source. But I also wanted to convey that the columns in the prefix are matched sequentially:
ignore_trailing_extra_source_columns_match_by_sequence
| node.query(f"DROP TABLE IF EXISTS {iceberg}") | ||
|
|
||
|
|
||
| def test_export_part_with_materialized_column(cluster): |
There was a problem hiding this comment.
Just to better understanding. Are this and next tests related to changes in current PR somehow? ignore_extra_source_columns_by_position is not used here, and tests cover different features.
There was a problem hiding this comment.
Yes, most of tests in this file are additional. However, some tests (eg test_export_part_ignore_extra_setting_drops_trailing_alias_column) show that simple resizing columns_to_read isn't enough.
Closes: #1717
Problem.
EXPORT PART/EXPORT PARTITIONmatched source and destination columns positionally, exactly likeINSERT INTO dest SELECT * FROMsrc, and required the column count to match exactly. This is unnecessarily strict for a commonreal scenario: a MergeTree table's schema grows over time (new trailing columns added), but older partitions still need to be exported to an external table (Iceberg/object storage) whose schema was fixed when it was created. Such exports were rejected outright with
NUMBER_OF_COLUMNS_DOESNT_MATCH, with no way to just drop the newer columns and export the rest.Solution. Added the export_merge_tree_part_schema_mismatch_mode setting (default strict, preserving the old behavior). The new ignore_extra_source_columns_by_position mode allows the source to have more columns than the destination: the extra trailing source columns (by declared/readable position, the same order
INSERT SELECTuses) are dropped, both at synchronous validation time (verifyExportSchemaCastable, so a real mismatch still fails immediately atALTER TABLE ... EXPORTtime) and at actual data-export time (ExportPartTask::addExportConvertingActions, via a projection step before the existing positional CAST). The reverse direction — destination has more columns than source — is still always rejected in both modes.Known follow-up. Ignored columns are still fully read and decompressed from disk before being discarded in the projection step, so no I/O is saved. Skipping the read up front isn't safe as a naive truncation: a kept
ALIAS/MATERIALIZED column can legally forward-reference an ignored one, so it would need a real dependency-graph walk instead. Left as a possible future optimization
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Added the export_merge_tree_part_schema_mismatch_mode setting. With ignore_extra_source_columns_by_position (default: strict), EXPORT PART/EXPORT PARTITION allows a source table with extra trailing columns — they are simply
ignored instead of failing with NUMBER_OF_COLUMNS_DOESNT_MATCH.
Documentation entry for user-facing changes
...
CI/CD Options
Exclude tests:
Regression jobs to run: