Skip to content

Fix upsert after schema evolution (#3105) - #3865

Closed
hedger9487 wants to merge 1 commit into
apache:mainfrom
hedger9487:fix/upsert-schema-evolution-3105
Closed

hedger9487 wants to merge 1 commit into
apache:mainfrom
hedger9487:fix/upsert-schema-evolution-3105

Conversation

@hedger9487

Copy link
Copy Markdown
Contributor

Closes #3105

Rationale for this change

When an Iceberg table undergoes schema evolution (e.g. update_schema().add_column(...) or update_schema().union_by_name(...)), calling table.upsert() previously failed with:

ValueError: Target schema's field names are not matching the table's field names: ['id', 'name', 'age', 'city', 'ping'], ['id', 'name', 'age', 'city']

Root Cause

In pyiceberg/table/upsert_util.py (get_rows_to_update), Step 1 previously cast the entire source_table (which has the new schema including evolved columns) to target_table.schema (which only contains columns present in older data files). PyArrow requires column names to match exactly when casting an entire table, raising ValueError.

Fix

  1. In Step 1, cast only the join_cols to the target table's join schema (join_schema = pa.schema([target_table.schema.field(col) for col in join_cols])).
  2. In Step 4, safely retrieve non-key values from target_row as None if the column was added during schema evolution and absent in target_table.
  3. Add regression tests covering upsert after add_column, union_by_name, multiple schema evolutions with composite keys, and no-op null comparisons, while explicitly verifying that underlying Parquet data files are properly replaced (Copy-on-Write) and snapshot operations include OVERWRITE and APPEND.

Are these changes tested?

Yes:

  • Added test_upsert_after_schema_evolution (verifies file replacement and snapshot operations)
  • Added test_upsert_after_schema_evolution_union_by_name
  • Added test_upsert_after_multiple_schema_evolutions_with_composite_keys
  • Added test_upsert_after_schema_evolution_noop_and_nulls
  • Verified mutation test fails without the fix and passes with the fix.
  • All 12 pre-commit linters and 471 table tests pass cleanly.

Are there any user-facing changes?

No.

* Cast only join_cols schema instead of full table schema in get_rows_to_update
* Safely handle missing non-key columns in target_table when comparing rows
* Add regression tests for upsert after add_column and union_by_name schema evolution
* Verify underlying Parquet file replacement and snapshot operations
Copilot AI lite review requested due to automatic review settings August 26, 2026 15:15

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

@github-actions github-actions Bot added the stale label Sep 26, 2026
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions Bot closed this Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upsert fails after update_schema().union_by_name() due to schema mismatch

2 participants