Repository navigation
[fix](iceberg) Promote equality-delete keys to current schema type - #68267
Merged
Gabriel39 merged 1 commit intoSep 22, 2026
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Iceberg equality deletes were compared in the delete file's physical type. A NULL key written under an old INT schema therefore narrowed an out-of-range LONG data value into NULL and deleted that row as well, and strict cast made the whole scan fail. Read the delete key, the data key and the delete side in the promoted type of the current snapshot schema instead, and promote historical delete values into that domain. With the comparison done in the wider type the cast is always lossless, so a non-NULL value can no longer become NULL. Add a unit test whose data file holds a value beyond the old INT range.
hubgeter
force-pushed
the
fix/iceberg-equality-delete-promotion
branch
from
September 20, 2026 09:22
757f0ff to
65a8453
Compare
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
|
Codex automated review failed and did not complete. Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Sep 23rd, 2026 6:44 AM. The selected account is excluded until 2026-09-23T06:44:00Z. Please trigger /review again; another configured account may be available. |
Contributor
TPC-H: Total hot run time: 28116 ms |
Contributor
TPC-DS: Total hot run time: 152091 ms |
Contributor
ClickBench: Total hot run time: 23.96 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
|
/review |
Contributor
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Request changes: the promoted comparison domain is the right fix for a versioned FileScannerV2 plan, and the added Parquet unit fixture captures the central overflow-to-NULL failure, but two supported compatibility paths can still return different or incorrect results.
Critical checkpoint conclusions:
- Goal and proof: the v2 delete/data expressions converge on one widened key type, but the implementation does not accomplish the goal for marker-less old-FE plans or for the selectable legacy scanner. The test proves only a semantics-v2, direct FileScannerV2 Parquet case.
- Scope and clarity: the two-file patch is focused and its widening comments are clear, but the parallel legacy implementation is left unchanged and the schema lookup lacks the required negotiation condition.
- Concurrency and lifecycle: no new thread or lock-order hazard was found. The equality-filter cache is owned by one FileScanLocalState, creation is shard-serialized, and cached typed blocks/hash indexes are immutable for the operator lifetime.
- Error, memory, null, and data correctness: v2 present/missing/nested branches and Parquet/ORC readers consistently target filter.key_types, and cast/prepare/open failures propagate. NULL-safe equality is exactly why narrowing overflow is dangerous; the two inline findings cover the remaining result-correctness gaps. No new ownership or memory-safety issue was found.
- Configuration and compatibility: no configuration or protocol field is added. The existing iceberg_scan_semantics_version marker must gate the result-changing behavior during rolling upgrades, and enable_file_scanner_v2=false remains a supported result-parity path.
- Parallel paths and conditions: current v2 Parquet/ORC, BY_NAME, missing/default, nested, dropped-field, strict/non-strict, and cache-key paths were traced. The unchanged scanner-v1 mixin is the uncovered parallel path.
- Testing and results: add a no-marker legacy-plan regression and an enable_file_scanner_v2=false parity regression, covering Parquet/ORC as practical. The new expected v2 rows are correct by static trace. Per the review-task constraint, I did not run builds or tests; existing author/CI reports were not treated as independent validation.
- Observability: existing cache hit/miss and memory counters remain intact; this local comparison change does not require new logging or metrics.
- Persistence, transactions, writes, and FE-BE variables: not applicable; there are no persistence/data-write changes or new transmitted fields.
- Performance: promotion occurs once while loading a delete file and the result remains cached; no new row-hot-path regression was found.
- User focus: no additional focus was supplied. The full changed-file and candidate sweep found no issue beyond the two inline blockers.
Gabriel39
approved these changes
Sep 22, 2026
Contributor
|
PR approved by anyone and no changes requested. |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions Bot
pushed a commit
that referenced
this pull request
Sep 22, 2026
…68267) ### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: Iceberg equality deletes are compared in the physical type of the delete file. When a column is legally promoted (for example INT to LONG), the data file can hold values outside the old type, but the data key was cast into that old type. An out-of-range value becomes NULL and then matches a NULL equality-delete key through NULL-safe equality, so a valid row is silently deleted. With `enable_strict_cast` the same cast makes the whole scan fail with `Value ... out of range for type int`. Resolve the comparison type from the current snapshot schema instead of the delete file, and promote historical delete values into that domain. With both sides in the wider type the cast is always lossless for an Iceberg-legal type promotion, so a non-NULL key can no longer turn into NULL. Include the current schema id in the delete-file cache key because the loaded block now depends on the comparison type. Covered by a new BE unit test: a data file holding LONG `NULL, 0, 1, 4294967296` plus a NULL equality-delete key written under the old INT schema. Before the fix the scan returned `{0, 1}`; after the fix it returns `{0, 1, 4294967296}`. ### Release note Fix Iceberg equality deletes being evaluated in the delete file's historical type, which could silently drop valid rows after a column type promotion. ### Check List (For Author) - Test - [ ] Regression test - [x] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason - Behavior changed: - [ ] No. - [x] Yes. - Does this need documentation? - [x] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
This was referenced Oct 27, 2026
morningman
added a commit
to morningman/doris
that referenced
this pull request
Oct 28, 2026
…lete and data file types ### What problem does this PR solve? Issue Number: None Related PR: apache#69536, apache#68267 Problem Summary: **In short.** FileScannerV2 now compares an Iceberg equality-delete key in the wider of the delete file's and the data file's own key types, instead of in the one table type the FE sends for the whole scan. For some table histories that single type is wrong: the reader then deletes rows it must keep, keeps rows it must delete, or fails the scan. This follows the review of apache#69536, where these cases came up. **Background.** An equality delete removes a data row whose key values equal a tombstone's. After a type promotion the two sides can be stored in different types: tombstones written before an INT -> LONG promotion hold INT values that must still match LONG data. apache#68267 made FileScannerV2 compare in the current schema's type, and for a key that the current schema no longer has, the FE sends a type recovered from the retained schemas (apache#69536). The reader casts both the tombstones and the data keys to that one type. **The problem, and what it cost.** One type per scan cannot describe every delete file: | History | Table type from the FE | Delete file | Data file | Before | After | |---|---|---|---|---|---| | `expireSnapshots().cleanExpiredMetadata(true)` removed the schema of a schema-only INT -> LONG promotion of a dropped key | INT | LONG 4294967296 | INT, one NULL key | the tombstone becomes NULL in INT and deletes the NULL-key row | row kept | | A read pinned before a later DATE -> TIMESTAMP_NS promotion of the key | TIMESTAMP_NS | DATE 1000-01-01 | DATE 1000-01-01 | scan fails: `TIMESTAMP_NS overflow when casting row ...` | row deleted | | No retained schema gives the key a concrete type, so the FE sends Iceberg `unknown` as STRING | STRING | TIMESTAMP_NS 2024-01-02 00:00 | DATE 2024-01-02 | `2024-01-02` != `2024-01-02 00:00:00.000000000`, the row stays visible | row deleted | | The delete was prepared under a TIMESTAMP_NS schema that the table later abandoned for TIMESTAMP | TIMESTAMP | TIMESTAMP_NS 12:00:00.000000500 | TIMESTAMP 12:00:00.000000 | the tombstone is rounded to microseconds and deletes the row | row kept | Three of the four return wrong rows without any error. The V1 reader is not changed by this PR. **How this PR fixes it.** - The delete block keeps the delete file's own key types. When the predicate for a data file is built, each key is compared in the wider of the delete file's type and the data file's type (`equality_delete_comparison_type`). The accepted widenings are the Iceberg promotions (INT -> LONG, FLOAT -> DOUBLE, decimal precision, DATE -> TIMESTAMP and DATE -> TIMESTAMP_NS), a wider fractional scale of the same date-time kind, wider integers, STRING -> VARBINARY (an Iceberg BINARY key decodes as either one, depending on `enable_mapping_varbinary`), and TIMESTAMP -> TIMESTAMP_NS. The last one is not an Iceberg promotion; it is accepted for the abandoned-schema case above, because scaling microseconds to nanoseconds is exact. Two types that convert in neither direction fail the scan instead of being compared in a third type. - The data keys are cast to that type. When it is the data file's wider type, the tombstones are widened once per scanner, and the converted delete block and its hash index are cached next to the original (`_convert_equality_delete_filter`). A table without type changes is compared as before, with no cast and no copy. - A key that the data file lacks is compared through its initial default, in the wider of the delete file's type and the table type. Without an initial default the key is NULL in that file whatever its table type, so it is compared in the delete file's type (an `unknown` key arrives as STRING). - The comparison casts are strict whatever the session's cast mode, through a new `strict` flag on `EvolutionCast`: a non-strict overflow would turn a key into NULL and let a NULL tombstone delete it. DATE -> TIMESTAMP_NS was already strict; other `EvolutionCast` users are unchanged. - The table types from the FE still resolve table fields and the defaults of missing keys; the delete-file cache key is unchanged. **Results.** - 9 new BE unit tests in `iceberg_reader_test.cpp`: the four histories above; INT tombstones against LONG, INT and LONG data files of one scan (the third split reuses the cached widened copy); a missing key's INT default against a LONG tombstone; a missing key without a default under an `unknown` table type; a microsecond key outside the nanosecond range, which must fail; and incomparable key types. With the reader before this PR, 6 of them fail; the other three pin behavior the old code shares. Removing the TIMESTAMP -> TIMESTAMP_NS widening fails 2, and making the casts non-strict fails 1. - End to end on a local cluster: a v2 table in a `file://` Hadoop catalog, built with the Iceberg 1.12.0 Java API: `k INT` with rows (1, NULL), (2, 0), (3, 1); `k` promoted to LONG without a snapshot; a delete of `k = 4294967296` written under that schema; `k` dropped; the delete committed; then `expireSnapshots().retainLast(2).cleanExpiredMetadata(true)`, which leaves schema 0 (`k INT`) and schema 2 (no `k`). `SELECT id` through FileScannerV2 returned 2, 3 before this PR and returns 1, 2, 3 with it. branch-4.1 and branch-4.2 have the same FileScannerV2 code. ### Release note Fix FileScannerV2 comparing Iceberg equality-delete keys in a single scan-wide type, which could delete or keep the wrong rows, or fail the scan, after a key's type changed (for example a schema-only promotion removed by `cleanExpiredMetadata`, or a key whose retained schemas only have it as `unknown`). ### Check List (For Author) - Test - [ ] Regression test - [x] Unit Test - `run-be-ut.sh --run --filter='IcebergV2ReaderTest.*:*EqualityDelete*:*ColumnMapper*:*EvolutionCast*'`: 273 of 273 pass on macOS arm64 (ASAN), including the 9 new cases. - [x] Manual test (add detailed scripts or steps below) - The local end-to-end check described under Results, with an ASAN BE built from this change. - [ ] No need to test or manual test. Explain why: - Behavior changed: - [ ] No. - [x] Yes. FileScannerV2 compares equality-delete keys per delete file and data file; two key types that convert in neither direction now fail the scan; the comparison casts are strict. - Does this need documentation? - [x] No. - [ ] Yes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Oct 29, 2026
#69691) ### What problem does this PR solve? Issue Number: None Related PR: #66538 (branch-4.1), #68267 Problem Summary: **In short.** This brings the V1-reader part of branch-4.1 #66538 (Iceberg V3 default values) to master: Iceberg equality deletes keep working after the type of their key column is promoted (INT to BIGINT, FLOAT to DOUBLE, a wider DECIMAL precision). It does not copy branch-4.1's approach, which compares a key in the delete file's older type; it compares in the scan schema's type, as #68267 already does in the V2 reader. **Background.** Iceberg allows widening a column's type, and files keep the type they were written with: an equality delete written while `id` was `int` stores its key as INT32, and so does a data file written then. The V1 reader (`enable_file_scanner_v2 = false`) loads each delete file into a delete block and compares it with the data rows, which the Parquet and ORC readers already return in the table's current type. **The problem, and what it cost.** The V1 reader compared every key in the type of the delete file it came from. After a promotion, any of these fails: | Case (V1 reader, `id` or `payload.existing` promoted from INT to BIGINT) | master | |---|---| | `SELECT id`, delete written before the promotion | Query fails: `Bad cast from type ColumnVector<BIGINT> to ColumnVector<INT>` | | `SELECT id`, one delete written before the promotion and one after | Query fails: the INT and BIGINT keys are merged into one delete block (debug builds abort on the block's type check) | | `SELECT v` (key not projected), same two deletes | Same | | `SELECT id`, delete on `payload.existing` written after the promotion, data file written before it | The INT values of the old file are copied into a BIGINT column: debug builds abort with `Bad cast`, release builds read the wrong bytes and keep or delete the wrong rows | branch-4.1 / branch-4.2 handle these by casting the data key down to the delete file's type. That narrowing is what #68267 fixed in the V2 reader: a data file written after the promotion can hold 4294967296, which becomes NULL as INT and is then deleted by the NULL key of a delete file written before the promotion (equality deletes use null-safe equality); with `enable_strict_cast` the whole scan fails instead. **How this PR fixes it.** - The type a key is compared in is the field's type in the scan schema (`equality_delete_key_type`, the same rule as V2's `restore_current_primitive_type`). A plan without a scan schema keeps the delete file's type, as before, and STRING / VARBINARY keys keep theirs too, since they match by bytes. - Delete keys, and nested keys extracted from the physical root of an older data file, are widened to that type with `converter::ColumnTypeConverter`, the converter the V1 Parquet and ORC readers already apply to the data column of a promoted field, so both sides hold the same value (`widen_equality_delete_key`). `NestedEqualityDeleteColumn` records the leaf's source type for that. - Delete blocks and hidden key carriers are built in the comparison type, so deletes written before and after a promotion share one block. **Results.** | Case (V1 reader) | master | This PR | |---|---|---| | `SELECT id`, INT delete `id = 2`, data 1, 2, 3 | `Bad cast` | 1, 3 | | `SELECT id`, INT delete `id = 2` + BIGINT delete `id = 3` | aborts (debug) / fails | 1 | | `SELECT v`, same two deletes | aborts (debug) / fails | 10 | | `SELECT id`, BIGINT delete `payload.existing = 20` on an old INT file | `Bad cast` (debug) / wrong rows (release) | 1, 3 | | `SELECT id`, data `NULL, 0, 1, 4294967296`, INT delete holding NULL | `Bad cast` (branch-4.x: 0, 1) | 0, 1, 4294967296 |
Gabriel39
pushed a commit
that referenced
this pull request
Oct 29, 2026
…lete and data file types (#69711) ### What problem does this PR solve? Issue Number: None Related PR: #69536, #68267 Problem Summary: **In short.** FileScannerV2 now compares an Iceberg equality-delete key in the wider of the delete file's and the data file's own key types, instead of in the one table type the FE sends for the whole scan. For some table histories that single type is wrong: the reader then deletes rows it must keep, keeps rows it must delete, or fails the scan. This follows the review of #69536, where these cases came up. **Background.** An equality delete removes a data row whose key values equal a tombstone's. After a type promotion the two sides can be stored in different types: tombstones written before an INT -> LONG promotion hold INT values that must still match LONG data. #68267 made FileScannerV2 compare in the current schema's type, and for a key that the current schema no longer has, the FE sends a type recovered from the retained schemas (#69536). The reader casts both the tombstones and the data keys to that one type. **The problem, and what it cost.** One type per scan cannot describe every delete file: | History | Table type from the FE | Delete file | Data file | Before | After | |---|---|---|---|---|---| | `expireSnapshots().cleanExpiredMetadata(true)` removed the schema of a schema-only INT -> LONG promotion of a dropped key | INT | LONG 4294967296 | INT, one NULL key | the tombstone becomes NULL in INT and deletes the NULL-key row | row kept | | A read pinned before a later DATE -> TIMESTAMP_NS promotion of the key | TIMESTAMP_NS | DATE 1000-01-01 | DATE 1000-01-01 | scan fails: `TIMESTAMP_NS overflow when casting row ...` | row deleted | | No retained schema gives the key a concrete type, so the FE sends Iceberg `unknown` as STRING | STRING | TIMESTAMP_NS 2024-01-02 00:00 | DATE 2024-01-02 | `2024-01-02` != `2024-01-02 00:00:00.000000000`, the row stays visible | row deleted | | The delete was prepared under a TIMESTAMP_NS schema that the table later abandoned for TIMESTAMP | TIMESTAMP | TIMESTAMP_NS 12:00:00.000000500 | TIMESTAMP 12:00:00.000000 | the tombstone is rounded to microseconds and deletes the row | row kept | Three of the four return wrong rows without any error. The V1 reader is not changed by this PR. **How this PR fixes it.** - The delete block keeps the delete file's own key types. When the predicate for a data file is built, each key is compared in the wider of the delete file's type and the data file's type (`equality_delete_comparison_type`). The accepted widenings are the Iceberg promotions (INT -> LONG, FLOAT -> DOUBLE, decimal precision, DATE -> TIMESTAMP and DATE -> TIMESTAMP_NS), a wider fractional scale of the same date-time kind, wider integers, STRING -> VARBINARY (an Iceberg BINARY key decodes as either one, depending on `enable_mapping_varbinary`), and TIMESTAMP -> TIMESTAMP_NS. The last one is not an Iceberg promotion; it is accepted for the abandoned-schema case above, because scaling microseconds to nanoseconds is exact. When the table field is `timestamptz`, a `DATETIMEV2` key (Parquet INT96, which carries no instant annotation, as Spark and Hive write it) and a `TIMESTAMPTZ` key compare as `TIMESTAMPTZ` at the larger scale, the `DATETIMEV2` side converted with the session time zone as the projection converts that column. When the table field is the STRING placeholder the FE sends for an Iceberg TIME, which Doris cannot represent, an ORC TIME key (a LONG annotated `iceberg.long-type=TIME`, read as `BIGINT`) and a Parquet one (`TIME(MICROS)`, read as `TIMEV2(6)`) compare as `TIMEV2(6)`, the `BIGINT` side converted exactly as microseconds from midnight by a dedicated expression (`CAST(BIGINT AS TIME)` reads the integer's digits as HHMMSS), and a value outside 00:00:00..23:59:59.999999 fails the scan. Two types that convert in neither direction fail the scan instead of being compared in a third type. - The data keys are cast to that type. When it is the data file's wider type, the tombstones are widened once per scanner, and the converted delete block and its hash index are cached next to the original (`_convert_equality_delete_filter`). A table without type changes is compared as before, with no cast and no copy. - A key that the data file lacks is compared through its initial default, in the wider of the delete file's type and the table type. When that default is NULL (no initial default, or a struct default `{}` whose leaf has none) the key is NULL in that file whatever its table type, so it is compared in the delete file's type (an `unknown` key arrives as STRING). A non-NULL default behind the STRING placeholder of an Iceberg TIME is Iceberg's text of the value (`00:00`), with at most six fractional digits, so it is parsed exactly in the tombstones' type, `TIMEV2` (`TIMEV2(6)` against an ORC TIME tombstone), by the strict comparison cast. A non-NULL default of a `timestamptz_ns` key fails the scan with `NotSupported`: V2 reads its tombstones as `TIMESTAMPTZ(6)` (Parquet floors the nanoseconds, ORC rounds them), so no parse of the nanosecond default compares exactly; the default epoch would match a tombstone 100 ns later and delete every row of the file. - `AncestorNullDefaultExpr`, which gives a missing child the NULL of its NULL parent and its default otherwise, now opens its children. Its `open()` returned before opening them, because `prepare()` had already set the flag its guard checks, so a default cast to the comparison type below it was never opened and aborted the BE with `Check failed: _open_finished` in `Cast`. - The comparison casts are strict whatever the session's cast mode, through a new `strict` flag on `EvolutionCast`: a non-strict overflow would turn a key into NULL and let a NULL tombstone delete it. DATE -> TIMESTAMP_NS was already strict; other `EvolutionCast` users are unchanged. - A delete file without rows is skipped before its keys are resolved: it deletes nothing, but the predicate evaluates its keys first, so its strict casts could fail the scan (a DATE key 1000-01-01 against an empty TIMESTAMP_NS delete file). - The table types from the FE still resolve table fields and the defaults of missing keys; the delete-file cache key is unchanged. **Results.** - 14 new BE unit tests in `iceberg_reader_test.cpp`: the four histories above; INT tombstones against LONG, INT and LONG data files of one scan (the third split reuses the cached widened copy); a missing key's INT default against a LONG tombstone; a missing key without a default under an `unknown` table type; a microsecond key outside the nanosecond range, which must fail; and incomparable key types. With the reader before this PR, 6 of them fail; the other three pin behavior the old code shares. Removing the TIMESTAMP -> TIMESTAMP_NS widening fails 2, and making the casts non-strict fails 1. Three more came from review: an INT96 data key against an adjusted-UTC tombstone in Asia/Shanghai, a NULL leaf under a struct default `{}`, and an empty TIMESTAMP_NS delete file against a DATE key; removing the INT96 rule, the NULL-leaf walk or the empty-file skip fails its test. Two more came from the third round: Iceberg TIME keys across all four ORC / Parquet data and delete file pairs, with NULL and non-NULL tombstones, and a nested Iceberg TIME key added with a text default, against Parquet TIME and ORC TIME tombstones, under NULL and non-NULL parents. Removing the TIME pair rule or the default parsing, or converting the ORC TIME with a SQL cast, fails them; leaving `AncestorNullDefaultExpr`'s children unopened aborts. One more came from the fourth round: a nested `timestamptz_ns` key with the defaults `...0000001`, the epoch and `...0000005` fails against an epoch, a 100 ns and a 500 ns tombstone, while a key without a default still matches NULL tombstones only; parsing those defaults again fails it. - End to end on a local cluster: a v2 table in a `file://` Hadoop catalog, built with the Iceberg 1.12.0 Java API: `k INT` with rows (1, NULL), (2, 0), (3, 1); `k` promoted to LONG without a snapshot; a delete of `k = 4294967296` written under that schema; `k` dropped; the delete committed; then `expireSnapshots().retainLast(2).cleanExpiredMetadata(true)`, which leaves schema 0 (`k INT`) and schema 2 (no `k`). `SELECT id` through FileScannerV2 returned 2, 3 before this PR and returns 1, 2, 3 with it. branch-4.1 and branch-4.2 have the same FileScannerV2 code. ### Release note Fix FileScannerV2 comparing Iceberg equality-delete keys in a single scan-wide type, which could delete or keep the wrong rows, or fail the scan, after a key's type changed (for example a schema-only promotion removed by `cleanExpiredMetadata`, or a key whose retained schemas only have it as `unknown`). ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [x] Unit Test - `run-be-ut.sh --run --filter='IcebergV2ReaderTest.*:*EqualityDelete*:*ColumnMapper*:*EvolutionCast*:*TableReader*'`: 526 of 526 pass on macOS arm64 (ASAN), including the 14 new cases. - Rebased onto master `43f9ceb48d7`: #69639 moved the strict-mode override of `EvolutionCast` from `prepare()` into `configure_cast_options()`, so the new `strict` flag now sets `CastOptions::strict_mode` there, next to the existing DATE -> TIMESTAMP_NS rule. The run above is on the content before the rebase (the third-round commit applies to both unchanged); the 67 TUs that this PR changes or that master changed since `5caaeb3e3d4` were syntax-checked against the rebased tree with the UT build flags, and CI runs the suites again. - [x] Manual test (add detailed scripts or steps below) - The local end-to-end check described under Results, with an ASAN BE built from this change. - [ ] No need to test or manual test. Explain why: - Behavior changed: - [ ] No. - [x] Yes. FileScannerV2 compares equality-delete keys per delete file and data file; two key types that convert in neither direction now fail the scan; the comparison casts are strict. - Does this need documentation? - [x] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into --> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
Iceberg equality deletes are compared in the physical type of the delete file.
When a column is legally promoted (for example INT to LONG), the data file can
hold values outside the old type, but the data key was cast into that old type.
An out-of-range value becomes NULL and then matches a NULL equality-delete key
through NULL-safe equality, so a valid row is silently deleted. With
enable_strict_castthe same cast makes the whole scan fail withValue ... out of range for type int.Resolve the comparison type from the current snapshot schema instead of the
delete file, and promote historical delete values into that domain. With both
sides in the wider type the cast is always lossless for an Iceberg-legal type
promotion, so a non-NULL key can no longer turn into NULL. Include the current
schema id in the delete-file cache key because the loaded block now depends on
the comparison type.
Covered by a new BE unit test: a data file holding LONG
NULL, 0, 1, 4294967296plus a NULL equality-delete key written under the old INT schema. Before the fix
the scan returned
{0, 1}; after the fix it returns{0, 1, 4294967296}.Release note
Fix Iceberg equality deletes being evaluated in the delete file's historical
type, which could silently drop valid rows after a column type promotion.
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)