[fix](iceberg) Fix historical scans after schema evolution - #67479
[fix](iceberg) Fix historical scans after schema evolution#67479Gabriel39 wants to merge 1 commit into
Conversation
Upgrade Iceberg and its shaded catalog dependency so time-travel predicates are resolved against historical schemas. Keep the Doris DeleteFileIndex fork aligned with Iceberg 1.11.0 and add rename/drop regression coverage.\n\nIssue: DORIS-28397
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: four distinct issues remain on the reviewed head.
Predicate/index correctness: the regression advances the snapshot and misses the schema-only/current-snapshot failure, while the default-enabled Doris batch-mode preflight still binds the historical predicate against current-schema specs and aborts production scans.
Cache behavior and observability/performance: the public custom-planner builder cannot receive Iceberg 1.11's full historical schema map, so dropped equality-delete keys repeatedly fail cache planning, warn, and fall back to native planning.
Dependency/catalog compatibility: the currently published shaded bytecode aligns with Iceberg 1.11 and null KMS preserves DLF's supported unencrypted path, but the runtime/package dependency is still a mutable cleanup-prone snapshot rather than an immutable release. No separate ABI, KMS, concurrency, lifecycle, security, position-delete, or deletion-vector issue was substantiated. Virtual-column and Parquet/ORC decode checkpoints are not implicated by this patch.
Test coverage: the added tests exercise only native planning after a later append and do not cover the failing schema-only, Doris preflight, or custom equality-delete cache paths. Builds/tests were not run because this review runner explicitly prohibits them.
User focus: no additional focus was provided; the entire PR was reviewed. Two bounded rounds converged with every reviewer reporting no new valuable findings. Review is complete for head 1849193.
| } else { | ||
| table.updateSchema().renameColumn("x", "renamed_x").commit(); | ||
| } | ||
| DataFile currentDataFile = DataFiles.builder(table.spec()) |
There was a problem hiding this comment.
[P1] Cover schema-only evolution before advancing the snapshot
Iceberg schema commits do not create a new snapshot, so immediately after this rename/drop historicalSnapshotId is still the table's current snapshot. In Iceberg 1.11, useSnapshot selects that snapshot's old schema, but SnapshotScan.specs() skips rebinding when the selected and current snapshot IDs are equal and returns specs bound to the renamed/dropped current schema. The old-name predicate then still fails in Projections. This append makes the IDs differ, so both tests avoid the unresolved case while the release note claims it is fixed. Please assert planning before this append and fix that path as well.
| IcebergUtils.convertToIcebergExpr(conjunct, scan.schema()); | ||
| Assert.assertNotNull(predicate); | ||
| scan = scan.filter(predicate); | ||
| Assert.assertEquals(1, materializeTasks(scan).size()); |
There was a problem hiding this comment.
[P1] Exercise the Doris planner, not only native planFiles
This assertion calls Iceberg's TableScan.planFiles() directly, but production reaches IcebergScanNode.isBatchMode() first and batch mode defaults on. That preflight calls getMatchingManifest(..., icebergTable.specs(), scan.filter()); these specs use the current renamed/dropped schema, so the historical old-name predicate fails while Projections binds it, and the catch at isBatchMode() rethrows. Thus these tests can pass while the default production query still fails. Rebind the preflight/custom-planner specs to scan.schema() (matching Iceberg 1.11's snapshot-spec behavior) and run this regression through the Doris node.
| return this; | ||
| } | ||
|
|
||
| Builder schemasById(Map<Integer, Schema> newSchemasById) { |
There was a problem hiding this comment.
[P2] Expose and wire the historical-schema map
The only Doris caller of the public iterable builder is in another package, so it cannot call this new package-private setter and currently supplies only icebergTable.specs(). Iceberg 1.11 added schemasById specifically because current specs no longer contain an equality-delete field after that field is dropped; fieldLookup then returns null and forDataFile throws. Doris catches that in the manifest-cache planner and reruns native planning, so every affected scan loses the enabled cache and logs a failure. Please make this hook public, pass the frozen table's full schemas() map, and cover a dropped equality key through the cache planner.
| </modules> | ||
| <properties> | ||
| <doris.hive.catalog.shade.version>3.1.2</doris.hive.catalog.shade.version> | ||
| <doris.hive.catalog.shade.version>3.1.3-ICEBERG-SNAPSHOT</doris.hive.catalog.shade.version> |
There was a problem hiding this comment.
[P1] Pin an immutable shade release before merging
This changes packaged/runtime consumers to 3.1.3-ICEBERG-SNAPSHOT, and this POM configures snapshots with updatePolicy=always. The coordinate currently resolves to timestamped build 3.1.3-ICEBERG-20260903.063704-1, but a later build can silently replace it and repository cleanup can make the same Doris revision stop resolving; there is no stable 3.1.3 artifact yet, and the PR description calls this temporary. Please publish the merged Doris Shade change and pin its immutable release here before merging.
2 participants
What problem does this PR solve?
Issue Number: DORIS-28397
Related PR: apache/doris-shade#63
Problem Summary:
Iceberg time-travel planning can fail when a predicate references a column that was renamed or dropped after the selected snapshot. Iceberg 1.11.0 includes the upstream fix that resolves historical partition specs with the correct schema. This PR upgrades the direct and shaded Iceberg dependencies, adapts the DLF table operations constructor, and synchronizes Doris's public
DeleteFileIndexfork with Iceberg 1.11.0 while retaining Java 8 source compatibility.The shaded catalog dependency temporarily uses
3.1.3-ICEBERG-SNAPSHOTuntil the corresponding Doris Shade change is released.Release note
Fix Iceberg time-travel queries after column rename or drop.
Check List (For Author)
Test
Validation performed:
IcebergScanNodeTest: 93 tests passed, including new rename and drop casesBehavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)