Skip to content

[AURON #2375] Fix Iceberg changelog scan field-id projection#2376

Open
lyne7-sc wants to merge 5 commits into
apache:masterfrom
lyne7-sc:fix/iceberg_changelogscan_fieldid
Open

[AURON #2375] Fix Iceberg changelog scan field-id projection#2376
lyne7-sc wants to merge 5 commits into
apache:masterfrom
lyne7-sc:fix/iceberg_changelogscan_fieldid

Conversation

@lyne7-sc

@lyne7-sc lyne7-sc commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #2375

Rationale for this change

The regular native Iceberg scan path already passes Iceberg field IDs to the native reader, which makes top-level schema evolution such as column rename and drop-then-add safe for Parquet files.

The newer insert-only Iceberg changelog scan path also reads the underlying Parquet data files through the native reader, but it does not pass the same field-id mapping into the native scan plan yet. As a result, native Parquet schema matching falls back to column names on the changelog path.

This can return wrong results after Iceberg schema evolution. For example, after RENAME COLUMN, pre-rename files may read as null; after DROP + ADD of the same name, the newly added column may read data from the old dropped column.

What changes are included in this PR?

  • Extract field IDs from SparkChangelogScan's expected Iceberg schema.
  • Reuse the existing Iceberg rename/drop detection for changelog scans.
  • Pass changelog field IDs into IcebergScanPlan instead of Map.empty.
  • Keep nested rename/drop unsupported and make ORC changelog scans fall back after top-level rename/drop, consistent with the regular Iceberg scan path.
  • Add changelog scan integration tests for:
    • renamed columns resolved by field-id;
    • drop-then-add columns with the same name not reusing the dropped field-id.

Are there any user-facing changes?

Yes. Insert-only Iceberg changelog scans on renamed or drop-then-added Parquet columns now return correct results under the native scan. Unsupported cases continue to fall back to Spark. No API change.

How was this patch tested?

Added cases to AuronIcebergIntegrationSuite.

@weiqingy weiqingy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on — closing the field-id gap on the changelog path is a real correctness fix, and the two new tests are honest regression tests (each fails on the pre-fix Map.empty behavior and passes after), mirroring the existing file-scan coverage. A few questions inline.

@lyne7-sc

lyne7-sc commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @weiqingy for the reviews! All comments have been addressed.

@weiqingy

weiqingy commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Thanks @lyne7-sc — went through 7c69c06a and all three are resolved.

The shared inspectFieldIdSupport helper now carries the field-id extraction, rename/drop detection, and both asserts for the file-scan and changelog paths, parameterized by the two scan → … lookups — so a future field-id change lands on both paths at once. Since this bug came from those two paths drifting apart, folding them together is the durable fix.

The note on SparkChangelogScan's internal field-name reflection localizes the assumption and points the next Iceberg bump at the fallback, and the ORC top-level rename/drop comment now reads the same on both paths.

Nothing further from me — the changes look good. I'll leave the final call to the maintainers.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes incorrect results in the native insert-only Iceberg changelog scan path after Iceberg schema evolution (top-level rename and drop-then-add), by ensuring native Parquet schema resolution uses Iceberg field IDs (matching the regular native Iceberg scan behavior) and falling back when unsupported rename/drop scenarios exist for ORC.

Changes:

  • Plumbs fieldIdsByName into IcebergScanPlan for changelog scans (instead of Map.empty) and reuses existing rename/drop detection to gate ORC support.
  • Adds changelog-scan-specific field-id and schema-history extraction for SparkChangelogScan via reflective access (with existing fallback-on-failure behavior).
  • Adds integration tests covering renamed columns and drop-then-add-with-same-name behavior under native changelog scans.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
thirdparty/auron-iceberg/src/main/scala/org/apache/spark/sql/auron/iceberg/IcebergScanSupport.scala Reuses shared field-id/rename-drop inspection and passes field IDs into changelog IcebergScanPlan; aligns ORC fallback logic with regular scan path.
thirdparty/auron-iceberg/src/main/scala/org/apache/iceberg/spark/source/AuronIcebergSourceUtil.scala Adds changelog-scan helpers to extract expected field IDs and table for rename/drop detection (via reflection).
thirdparty/auron-iceberg/src/test/scala/org/apache/auron/iceberg/AuronIcebergIntegrationSuite.scala Adds native changelog integration tests for rename and drop-then-add correctness using field-id projection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Iceberg changelog scan returns wrong data after column rename / drop-then-add

3 participants