Core, Spark: Fix equality deletes non-deterministic schema ordering (#13873) by RussellSpitzer · Pull Request #15514 · apache/iceberg

RussellSpitzer · 2026-03-04T18:53:26Z

Equality delete schemas constructed in DeleteFilter.applyEqDeletes relied on the field order of requiredSchema, which varies depending on the query's projection. When the SparkExecutorCache returned delete records read with one field ordering to a reader expecting another, StructProjection silently misinterpreted the positional data, causing deletes to be skipped.

We fix this by Canonicalize the deleteSchema by sorting fields by field ID. Now every reader produces the same schema for deletes regardless of projection, ensuring cache hits return correctly ordered records.

Coded with the help of Cursor and claude-4.6.opus-high

--
TLDR;

First call read with schema [a,b] for deletes:
read+project(file, [a,b]) → cache_projected_deletes(file) → applyDeletes with [a,b]

Second call reads with Schema [b, a] for deletes:
load_cache_projected_deletes(file) → applyDeletes with [b,a] → mismatch

Fixes #13873

…hema ordering (apache#13873) Equality delete schemas constructed in DeleteFilter.applyEqDeletes relied on the field order of requiredSchema, which varies depending on the query's projection. When the SparkExecutorCache returned delete records read with one field ordering to a reader expecting another, StructProjection silently misinterpreted the positional data, causing deletes to be skipped. We fix this by Canonicalize the deleteSchema by sorting fields by field ID. Now every reader produces the same schema for deletes regardless of projection, ensuring cache hits return correctly ordered records. Coded with the help of Cursor and claude-4.6.opus-high

mxm

LGTM. Looks like the cache key is the delete file itself, which makes sense, as long as we use a deterministic approach for the schema projection.

pvary · 2026-03-06T11:56:07Z

+    eqTestTable.newRowDelta().addDeletes(eqFile).commit();
+
+    String tableRef = TableIdentifier.of("default", EQ_CACHE_TABLE).toString();
+    int expectedRows = 7;


nit: maybe a little bit easier to understand?

Suggested change

int expectedRows = 7;

int expectedRows = data.size() - delete.size();

pvary · 2026-03-06T12:14:25Z

+            Types.NestedField.optional(1, "id", Types.IntegerType.get()),
+            Types.NestedField.optional(2, "a", Types.IntegerType.get()),
+            Types.NestedField.optional(3, "b", Types.IntegerType.get()));
+    PartitionSpec spec = PartitionSpec.builderFor(eqDeleteTestSchema).bucket("id", 1).build();


Why do we not use an unpartitioned table here?

No reason, I originally had this in a different file where all the tables were partitioned and I didn't want to feel left out. Now that it's here we can remove it

pvary

Marked a few places where the code could made cleaner

stevenzwu

LGTM. thanks for adding the unit test. just a couple of nit comments

rdblue · 2026-03-06T19:14:11Z

+   * order than the table schema, which can cause different deleteSchema orderings, poisoning the
+   * cache.
+   */
+  @TestTemplate


The actual test makes sense, but I don't see why it is necessary to run this test of a core feature (DeleteFilter) in Spark. This should be done in the tests for that class.

Main reason is I couldn't find a place to fit it in any of those places. I did try but it basically involved writing a custom test cache implementation as well. So while it's possible it basically involves re-implementing everything to get the same behavior. We could add a unit test for sorting though, that's pretty easy

Yeah it looks like we currently don't have a great place for testing out just the logic within DeleteFilter. I think in the current structure, we'd have to have our own implementation of DeleteFilter for the test and as @RussellSpitzer said by the time we do all that it's basically just re-implementing what we have in the engine integrations like Spark. I think I agree with the current split of tests, but maybe the fact that we want to test DeleteFilter in isolation means we need to do some refactoring.

https://gist.github.com/sfc-gh-rspitzer/e53841222262010847a68aa2ed59dd39 < Here is what Claude thinks that should look like. I haven't really gone through it in depth I just figured if I was writing an implementation of DeleteFilter and DeleteLoader we are going way off track.

Basically we mock just about everything around the load, just to test that the schema is always canonicalized.

I didn't realize that the caching part was tied to Spark. This should be okay.

rdblue · 2026-03-06T19:15:45Z

@amogh-jahagirdar FYI. Here's another one for the next 1.10.x release.

amogh-jahagirdar

This looks right to me, thank you @RussellSpitzer !

amogh-jahagirdar · 2026-03-06T20:08:26Z

+   * order than the table schema, which can cause different deleteSchema orderings, poisoning the
+   * cache.
+   */
+  @TestTemplate


Yeah it looks like we currently don't have a great place for testing out just the logic within DeleteFilter. I think in the current structure, we'd have to have our own implementation of DeleteFilter for the test and as @RussellSpitzer said by the time we do all that it's basically just re-implementing what we have in the engine integrations like Spark. I think I agree with the current split of tests, but maybe the fact that we want to test DeleteFilter in isolation means we need to do some refactoring.

RussellSpitzer · 2026-03-09T15:03:11Z

@rdblue Any final thoughts? Are you onboard with the test even though I agree it is quite suboptimal. We may just want to raise another issue to refactor this whole bit of code into something more testable ...

amogh-jahagirdar · 2026-03-12T16:01:09Z

I'll go ahead and merge, thanks @RussellSpitzer and all for reviewing!

… schema ordering Equality delete schemas constructed in DeleteFilter.applyEqDeletes relied on the field order of requiredSchema, which varies depending on the query's projection. When the SparkExecutorCache returned delete records read with one field ordering to a reader expecting another, StructProjection silently misinterpreted the positional data, causing deletes to be skipped. We fix this by Canonicalize the deleteSchema by sorting fields by field ID. Now every reader produces the same schema for deletes regardless of projection, ensuring cache hits return correctly ordered records. Coded with the help of Cursor and claude-4.6.opus-high

…ed-it] [databricks] (#15964) **JaCoCo production line coverage: not fully measurable locally — no compatible Scala 2.13 / shim 411 nightly baseline is published.** Contributes to #12885. ### Description This test-only recovery restores enforced CPU/GPU result checks for mixed Iceberg position and equality deletes on Iceberg 1.10.2 and newer. Before this change, the format-v2 and format-v3 cases were permanent expected failures with fixed data seeds even on runtimes containing the upstream equality-delete cache fix. The affected older releases are now skipped instead of spending test time on known failures; production behavior is unchanged. [Apache Iceberg #15514](apache/iceberg#15514), released in 1.11.0 and [backported to 1.10.2](apache/iceberg#15605), fixes equality-delete records being cached in a query-dependent field order. This PR: - Skips both mixed-delete tests on Iceberg versions older than 1.10.2, removes their expected-failure markers and permanent seed overrides on fixed versions, and fails pytest configuration when `--iceberg` lacks an expected runtime version. - Passes the expected Iceberg version through the Spark 4.0 extra-classpath pre-merge launcher and documents the required environment variable. - Keeps every count, full-row, and GPU-scan assertion. For the format-v3 count query only, disables adaptive query execution so plan validation sees the completed scan, then resets session configuration before fixture cleanup. Older supported dependencies are not fixed by this PR, so #12885 should remain open. Iceberg 1.10.2 was policy-checked but was not GPU-runtime-tested locally. Shared pytest configuration and pre-merge launcher paths changed, so Databricks remains in CI scope. #### Validation - Spark 4.1.1 / Iceberg 1.11.0 / Python 3.10: all six focused format-v2/format-v3 CPU/GPU cases passed across the three reader modes (`6 passed, 45 deselected` in 219.85 seconds). Full-row equality and the format-v3 `GpuBatchScanExec` assertion were exercised. - Spark 4.0.0 / Iceberg 1.10.1 / Python 3.10: all six focused cases were selected and skipped (`6 skipped, 45 deselected` in 1.73 seconds). - Seven release-boundary checks and two decorator-policy checks passed. The checked cases cover missing version, 1.6.1, 1.9.2, 1.10.0, 1.10.1, 1.10.2, and 1.11.0. - Unset `EXPECTED_ICEBERG_VERSION` with `--iceberg` failed during pytest configuration with exit 4 and the expected error. `bash -n` and a focused launcher probe confirmed that the Spark 4.0 extra-classpath path forwards Iceberg 1.10.1. - Python 3.10 compilation and `git diff --check` passed. No production performance change is expected. AI assistance: The change and PR description were prepared with Codex assistance. ### Checklists Documentation - [ ] Updated for new or modified user-facing features or behaviors - [x] No user-facing change Testing - [x] Added or modified tests to cover new code paths - [ ] Covered by existing tests (Please provide the names of the existing tests in the PR description.) - [ ] Not required Performance - [ ] Tests ran and results are added in the PR description - [ ] Issue filed with a link in the PR description - [x] Not required --------- Signed-off-by: Allen Xu <allxu@nvidia.com>

github-actions Bot added spark data labels Mar 4, 2026

RussellSpitzer commented Mar 4, 2026

View reviewed changes

Comment thread data/src/main/java/org/apache/iceberg/data/DeleteFilter.java

mxm approved these changes Mar 5, 2026

View reviewed changes

pvary reviewed Mar 6, 2026

View reviewed changes

Comment thread spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkReaderDeletes.java Outdated

pvary approved these changes Mar 6, 2026

View reviewed changes

stevenzwu approved these changes Mar 6, 2026

View reviewed changes

Comment thread spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkReaderDeletes.java Outdated

Comment thread spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkReaderDeletes.java Outdated

Reviewer Comments

0a32bcf

huaxingao approved these changes Mar 6, 2026

View reviewed changes

rdblue reviewed Mar 6, 2026

View reviewed changes

Comment thread data/src/main/java/org/apache/iceberg/data/DeleteFilter.java Outdated

rdblue reviewed Mar 6, 2026

View reviewed changes

rdblue added this to the Iceberg 1.11.0 milestone Mar 6, 2026

Reviewer Comments

50bea09

github-actions Bot added the API label Mar 6, 2026

amogh-jahagirdar approved these changes Mar 6, 2026

View reviewed changes

Method Rename

1eefb28

RussellSpitzer mentioned this pull request Mar 10, 2026

Spark: Include delete files in scan task cache key #15564

Closed

RussellSpitzer added the bug Something isn't working label Mar 11, 2026

rdblue approved these changes Mar 12, 2026

View reviewed changes

amogh-jahagirdar merged commit f865bac into apache:main Mar 12, 2026
35 checks passed

amogh-jahagirdar mentioned this pull request Mar 12, 2026

Backport #15514 for Equality Delete Schema Ordering to 1.10 #15600

Closed

SergeiPatiakinEdb mentioned this pull request Apr 23, 2026

Equality deletes ignored on executor cache hit with different query #15039

Open

1 of 3 tasks

RussellSpitzer deleted the FixEqualityCacheIssue branch April 23, 2026 14:38

stevenzwu mentioned this pull request May 20, 2026

Docs: Add release notes for 1.11.0 #16431

Merged

qinghui-xu mentioned this pull request Jun 11, 2026

Exception when using DELETE FROM Spark query on table using copy-on-write #14239

Open

1 of 3 tasks

wjxiz1992 mentioned this pull request Sep 11, 2026

[SkipRecovery] Re-enable mixed-delete tests on Iceberg 1.10.2+ [reduced-it] [databricks] NVIDIA/cudf-spark#15964

Merged

3 of 8 tasks

	int expectedRows = 7;
	int expectedRows = data.size() - delete.size();

Conversation

RussellSpitzer commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mxm left a comment

Choose a reason for hiding this comment

Uh oh!

pvary Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

pvary Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RussellSpitzer Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pvary left a comment

Choose a reason for hiding this comment

Uh oh!

stevenzwu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rdblue Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

RussellSpitzer Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amogh-jahagirdar Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

RussellSpitzer Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdblue Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

rdblue commented Mar 6, 2026

Uh oh!

amogh-jahagirdar left a comment

Choose a reason for hiding this comment

Uh oh!

amogh-jahagirdar Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RussellSpitzer commented Mar 9, 2026

Uh oh!

amogh-jahagirdar commented Mar 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

RussellSpitzer commented Mar 4, 2026 •

edited

Loading

pvary Mar 6, 2026 •

edited

Loading

RussellSpitzer Mar 6, 2026 •

edited

Loading

RussellSpitzer Mar 6, 2026 •

edited

Loading