Skip to content

Spark: Remove Spark 2 test assumptions for write projection#15823

Merged
huaxingao merged 5 commits intoapache:mainfrom
Ruobing1997:remove-spark2-test-assumptions
Mar 31, 2026
Merged

Spark: Remove Spark 2 test assumptions for write projection#15823
huaxingao merged 5 commits intoapache:mainfrom
Ruobing1997:remove-spark2-test-assumptions

Conversation

@Ruobing1997
Copy link
Copy Markdown
Contributor

@Ruobing1997 Ruobing1997 commented Mar 30, 2026

Remove test Assumptions for Spark 2
Closes #15821

@github-actions github-actions bot added the spark label Mar 30, 2026
@Ruobing1997 Ruobing1997 force-pushed the remove-spark2-test-assumptions branch from 988f88d to 90ab25c Compare March 30, 2026 04:53
@manuzhang manuzhang changed the title Remove Spark 2 test assumptions for write projection Spark: Remove Spark 2 test assumptions for write projection Mar 30, 2026
@manuzhang
Copy link
Copy Markdown
Member

@Ruobing1997 I think there are more places where Spark 2 is assumed. Can you check and remove all of them?

@Ruobing1997
Copy link
Copy Markdown
Contributor Author

Of course! Let me take a look 👁️

@Ruobing1997
Copy link
Copy Markdown
Contributor Author

Ruobing1997 commented Mar 30, 2026

seems the partial column write is not supported 🤔 seems only supported in 4.1

@Ruobing1997 Ruobing1997 force-pushed the remove-spark2-test-assumptions branch from f5a629e to 7c9aeef Compare March 30, 2026 06:41
@@ -433,10 +427,6 @@ public void testPartitionedFanoutCreateWithTargetFileSizeViaOption2() {

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.

If these tests don't work on 3.4/3.5/4.0, should we remove them from those versions entirely rather than keeping dead tests? And why these tests only work with Spark2 and Spark4.1?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree with removing these tests... Seems dead code under the current logic 🤔

Copy link
Copy Markdown
Contributor Author

@Ruobing1997 Ruobing1997 Mar 30, 2026

Choose a reason for hiding this comment

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

And regarding

why these tests only work with Spark2 and Spark4.1?

I found this issue: https://issues.apache.org/jira/browse/SPARK-51290 for allowing partial column writes.
This fills default values for missing columns in DSv2 writes which seems why our tests pass on 4.1

And I have a question: I noticed this was enabled in Spark 4.1. Is there any chance this could be backported, or is it considered a new feature?

@Ruobing1997
Copy link
Copy Markdown
Contributor Author

Ruobing1997 commented Mar 31, 2026

Thanks @huaxingao for approving the PR. Are we good to merge it? ❤️
cc: @manuzhang

@huaxingao huaxingao merged commit 3ff0d97 into apache:main Mar 31, 2026
25 checks passed
@huaxingao
Copy link
Copy Markdown
Contributor

Thanks @Ruobing1997 for the PR! Thanks @manuzhang for the review!

@Ruobing1997
Copy link
Copy Markdown
Contributor Author

😋 Looking forward to contributing more!

ldudas-marx pushed a commit to ldudas-marx/iceberg that referenced this pull request Mar 31, 2026
…5823)

* Remove Spark 2 test assumptions for write projection

* apply formatting

* remove dead code for spark 2

* revert partial column fix for 3.4, 3.5 and 4.0

* remove spark 2 testing in 3.4, 3.5 and 4.0
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.

Remove tests assumption for Spark2

3 participants