Repository navigation
[fix](fe) Resolve dropped Iceberg equality-delete keys after a schema ID is reused - #69536
Open
morningman wants to merge 2 commits into
Open
morningman wants to merge 2 commits into
morningman wants to merge 2 commits into
Conversation
… ID is reused ### What problem does this PR solve? Issue Number: None Related PR: apache#66538, apache#69394 Problem Summary: When an Iceberg table drops a column so that its schema becomes equal to an earlier one, Iceberg reuses the earlier schema ID instead of adding a new schema, and the metadata schema list keeps that schema at its original, earlier position. The equality-delete schema carrier in the Iceberg connector located the scan schema in that list by ID and searched only the schemas listed before it. A live equality delete keyed on the dropped column therefore could not find the schema that still defines the key, and every scan of the table failed with "Iceberg equality-delete fields are absent from schema history". This happens in steady state, not only during an upgrade. Reproduce: create a table with column id, add column k, write data and an equality delete keyed on k, drop k (the schema returns to the initial one and reuses its ID), then query the table. branch-4.1 (apache#66538) searches the scan snapshot's parent chain first, so the definition recorded on that lineage wins, and falls back to the whole metadata list, newest first, for schema-only changes and expired ancestors. This change ports that order to IcebergScanPlanProvider. Each lineage schema is indexed at most once and the walk stops as soon as every key is resolved. It also ports the branch-4.1 tests for a reused schema ID (with and without an expired parent) and for time travel past a later rename. The existing test equalityCarrierKeepsLatestDefinitionOfDroppedEqualityKey (apache#69394) renamed and then dropped the key with no write in between. It now writes a snapshot after each schema change, as the branch-4.1 test it was ported from does, and still expects k2. ### Release note Fix Iceberg queries that failed with "equality-delete fields are absent from schema history" after a column used as an equality-delete key was dropped and the table schema returned to an earlier one. ### Check List (For Author) - Test: Unit Test - fe-connector-iceberg: 4 new cases and 1 adjusted case in IcebergScanPlanProviderTest; the two reused-ID cases fail without the fix; the whole module passes - Behavior changed: Yes. Such queries no longer fail. When a dropped key has several historical definitions, the carrier now prefers the one recorded on the scan snapshot's lineage, as branch-4.1 does. - Does this need documentation: No Co-authored-by: daidai <changyuwei@selectdb.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
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.
One P1 correctness regression is reported inline: resolving a dropped key solely from snapshot lineage can select a type older than a pending writer's schema, causing a widened equality-delete value to become NULL and match an unrelated NULL-key row. Changes are requested. No existing review threads or previously reported blockers were present.
Reviewed both changed files and the affected planning -> schema carrier -> scan-parameter -> native-reader flow at dc21748. Critical checkpoint conclusions:
- Goal and proof: the real-table tests address schema-ID reuse and expired parents, including dictionary transport. The pending-writer promotion case in the inline finding remains incorrect and lacks coverage.
- Scope and reuse: the change is focused on one lookup algorithm and its unit tests, reusing the existing historical-field merge helpers.
- Concurrency: new collections are local and reads use statement-frozen table metadata; no new locks or worker threads. The finding concerns external writes committing after schema-only changes, which the snapshot schema does not fully describe.
- Lifecycle: statement/table/FileIO ownership is unchanged. Null schema IDs and expired parents reach the schema-history fallback; SDK expiration retains schemas referenced by retained snapshots.
- Configuration: no configuration changes.
- Compatibility: no public SPI, storage-format or wire-format change. Existing backend-semantics fencing remains in use.
- Thrift/protobuf IDs: no gensrc changes; branch field-ID comparison is inapplicable.
- Parallel paths: production and testing callers pass the selected snapshot; normal, pinned/ref and streaming scans share the carrier. Both native reader generations were inspected; system-table handling is separate and unchanged.
- Conditions and errors: repeated schema IDs are deduplicated, the no-missing-key path skips ancestry, and missing schemas/unresolved IDs fail loudly. Early completion after resolving an older type causes the reported defect.
- Test coverage: four new JUnit tests and the amended rename/drop test were inspected alongside existing nested-key, default, reused-name and reader-promotion tests. Proxy histories establish local selection rules, not end-to-end reader results. No test directly covers the reported sequence or large missing-key ancestry.
- Test results: no generated result files changed. No builds or tests were run under the static-only review contract; the author's reported test count was not independently validated.
- Observability: existing error messages identify missing schema/equality IDs; no new logging or metrics requirement was substantiated for this local planning change.
- Transactions and persistence: no Doris transaction, EditLog or replay changes. External snapshot commit semantics were checked against the pinned Iceberg SDK sources linked inline.
- Writes and crash behavior: production changes only reconstruct read metadata; they introduce no writes, commit protocols or resource handles.
- FE/BE variables: only a local Java Snapshot argument is added; dictionary fields and transport paths remain unchanged. Its historical type affects the existing BE comparison contract as described inline.
- Performance: schema indexing is deduplicated and carrier size remains bounded by the current schema and relevant historical fields. Missing keys can require O(retained ancestry) lookups and REST refs-mode can load deferred snapshot history; no separate actionable performance regression was established or benchmarked.
- Other concerns: nested field-ID merging, dropped/re-added names, stable initial defaults, schema-only updates, time travel, branches, expiry, and both reader generations were checked. No additional verified finding remains.
No additional user-provided focus was supplied. Review limits are static inspection only, unexecuted runtime tests, and unmeasured large-lineage/REST history cost. All identified candidates and risk focuses have explicit dispositions in the shared ledger. Final convergence is recorded separately by the submission helper after the unchanged-payload reviewer pass.
Review converged after 3 round(s).
| throw new IllegalStateException( | ||
| "Iceberg snapshot schema " + schemaId + " is absent from table metadata"); | ||
| } | ||
| addHistoricalEqualityFields(fields, missing, lineageSchema); |
Contributor
There was a problem hiding this comment.
[P1] Preserve promoted types when recovering a dropped equality key
Snapshot lineage can omit the type used by a pending writer. Start with optional k INT and an existing row where k IS NULL; promote k to LONG without a snapshot, prepare a LONG equality-delete file for k=2147483648, drop k, then commit the pending RowDelta. Use unpartitioned files without column metrics (including null counts and bounds). The SDK permits this: SnapshotProducer.apply() refreshes metadata and stamps the current schema, and BaseRowDelta does not reject the changed key schema. The delete snapshot therefore records the dropped schema while its parent still records INT.
This call resolves the ID from that INT ancestor and removes it from missing, so the retained schema-only LONG definition is never considered. V2 _resolve_equality_delete_fields takes the carrier's INT as its comparison type; _load_equality_delete_file then casts the LONG tombstone to INT. Overflow becomes NULL in non-strict mode, and NULL-safe equality incorrectly deletes the existing NULL-key row (strict mode fails the query). The previous metadata-history lookup selected LONG and preserved that row. Preserve the applicable promoted comparison type when choosing lineage definitions, and cover the promotion/drop/pending-delete sequence in tests.
### What problem does this PR solve? Issue Number: None Related PR: apache#66538 Problem Summary: Review follow-up. The previous commit resolves an equality-delete key that the scan schema no longer has from the scan snapshot's parent chain first. With Iceberg 1.12 a pending RowDelta is stamped with the schema that is current when it commits, and nothing checks its equality field IDs against that schema. So for a key k promoted from INT to LONG by a schema-only change (no snapshot), a writer that wrote LONG tombstones, and k dropped before that delete commits, the delete snapshot records the schema without k and its parent the INT one: no snapshot on the lineage records LONG. The carrier then said INT, FileScannerV2 compares equality keys in the carrier's type, and a tombstone such as 2147483648 was cast to NULL in non-strict mode and deleted every row whose k is NULL (strict mode failed the query). The previous metadata-list lookup found LONG. A dropped key now keeps its lineage definition (name, optionality, initial default) but takes the widest type its field ID has in any retained schema. Field IDs are never reused and promotion is monotonic and lossless, so that type covers every file written for the field, also when it comes from a schema after a time-travel target. ### Release note None ### Check List (For Author) - Test: Unit Test - IcebergScanPlanProviderTest#equalityCarrierKeepsPromotedTypeOfKeyDroppedBeforeItsDeleteCommits replays the sequence with the SDK; it fails without the widening - fe-connector-iceberg module tests - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
Contributor
TPC-H: Total hot run time: 28841 ms |
Contributor
TPC-DS: Total hot run time: 150849 ms |
Contributor
TPC-H: Total hot run time: 28114 ms |
Contributor
TPC-DS: Total hot run time: 150394 ms |
Contributor
ClickBench: Total hot run time: 24.3 s |
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: None
Related PR: #66538, #69394
Problem Summary:
In short. This ports a fix from branch-4.1 #66538 (Iceberg V3 default values). When a dropped column is still referenced by a live equality delete, the Iceberg connector now looks up the column's definition along the scan snapshot's parent chain first, and only then in the whole schema list. Master searched only the part of the schema list before the scan schema. That breaks as soon as Iceberg reuses a schema ID, and every query on such a table failed.
Background. An equality delete names its key columns by field ID. If a key column has since been dropped, the scan schema no longer has it, so the connector adds the column's historical definition to the schema dictionary it sends to BE (
IcebergScanPlanProvider.schemaForPotentialEqualityDeletes). BE needs that field ID, type and initial default to read the key from data files. Master (#65851) took the definition from the metadata schema list, searching backwards from the scan schema's position, on the assumption that the list is in chronological order.The problem, and what it cost. Schema IDs are not a sequence. When a schema change produces a schema equal to an existing one, Iceberg reuses that schema's ID and does not append to the list. Dropping a column that was added after the table was created is the common case: the schema returns to the initial one, so the current schema is the first entry of the list, and the schema that still defines the key comes after it. The backward search never reaches that schema, and the scan fails.
Reproduce: create a table with
id; add columnk; write data and an equality delete keyed onk; dropk; query the table.Iceberg equality-delete fields are absent from schema history: [2]How this PR fixes it. It uses the same order as
IcebergScanNode.getSchemaFieldsForScanon branch-4.1 and branch-4.2. First, walk the scan snapshot's parent chain and take each missing key from the schema that snapshot was written with. Then search the whole metadata schema list, newest first, for keys that only schema-only changes or expired ancestors can explain. Unlike branch-4.x, the walk indexes each schema at most once and stops as soon as every key is resolved, so a long lineage that shares one schema costs only map lookups. When no key is missing, nothing changes and no snapshot is read, as before.When a dropped key has several historical definitions, the one recorded on the scan's lineage now wins, as on branch-4.x. The field ID and the initial default are the same either way. Only a name or optionality changed by a schema-only commit after the last write can differ. The existing test
equalityCarrierKeepsLatestDefinitionOfDroppedEqualityKey(from #69394) renamed and then dropped the key with no write in between. It now writes a snapshot after each schema change, as the branch-4.1 test it was ported from does, and still expectsk2.Results. This PR ports four tests from #66538 into
IcebergScanPlanProviderTest:equalityCarrierResolvesDroppedKeyWhenSchemaIdIsReuseduses a real in-memory table and also checks the scan dictionary thatgetScanNodePropertiesbuilds.equalityCarrierResolvesReusedSchemaIdAfterParentSnapshotsExpireis the same, with the snapshots that recorded the key expired.equalityCarrierPrefersScanLineageOverLaterRenameForTimeTravelandequalityCarrierKeepsDefaultWhenTimeTravelParentExpiredBeforeLaterRenameuse table doubles with schema histories that the SDK cannot build.With master's search, the two reused-ID tests fail with the error above. Without the lineage walk, the time-travel test gets
k2instead ofk. All 1693 tests of fe-connector-iceberg pass.Original author: @hubgeter
Release note
Fix: queries on an Iceberg table failed with "equality-delete fields are absent from schema history" after a column used as an equality-delete key was dropped and the table schema returned to an earlier one.
Check List (For Author)
Test
Behavior changed:
Such queries no longer fail. When a dropped equality-delete key has several historical definitions, the one recorded on the scan snapshot's lineage wins, as on branch-4.x.
Does this need documentation?
Check List (For Reviewer who merge this PR)