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

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

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

3 tasks

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

Uh oh!

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