Skip to content

Conversation

@eric-maynard
Copy link
Contributor

@eric-maynard eric-maynard commented Aug 18, 2025

This is a backport of c3d50e1, e667670, and 5d5e0a3, where tests were added for Spark 4.0 but not Spark 3.5.

@github-actions github-actions bot added the build label Aug 18, 2025
@eric-maynard eric-maynard marked this pull request as ready for review August 18, 2025 22:34
@kevinjqliu
Copy link
Contributor

Thanks for the PR!

For context, we're trying to make sure spark 3.5 and 4.0 have feature parity in the 1.10 release since this is the first release with Spark 4.0. More context in https://lists.apache.org/thread/8xzbg1wqft2grv8v1f13vb86vd8f7rjd

#13450 (comment)
#13391 (comment)

@kevinjqliu kevinjqliu added this to the Iceberg 1.10.0 milestone Aug 19, 2025
@kevinjqliu kevinjqliu self-requested a review August 19, 2025 04:14
@eric-maynard eric-maynard changed the title Backport & fix Parquet encoding tests for Spark 3.5 Backport Parquet encoding tests for Spark 3.5 Aug 25, 2025
@huaxingao
Copy link
Contributor

@eric-maynard Thanks for the back-port PR! Should we put these golden files in a common place so we don't have to add them for each Spark version?

@huaxingao
Copy link
Contributor

@RussellSpitzer @stevenzwu @nastra @kevinjqliu
We originally planned to support the delta encodings only in Spark 4.0. Since we’re backporting support to Spark 3.5, we should place the golden files in a common location to avoid duplicating them across Spark versions. I’m thinking we can add these golden files under iceberg-parquet/src/test/parquet. Any thoughts?

@stevenzwu
Copy link
Contributor

How about iceberg-parquet/src/test/resources?

@stevenzwu stevenzwu removed this from the Iceberg 1.10.0 milestone Aug 29, 2025
@stevenzwu
Copy link
Contributor

I removed this one from the 1.10.0 milestone as it is just test as @kevinjqliu says

@eric-maynard
Copy link
Contributor Author

eric-maynard commented Aug 29, 2025 via email

@kevinjqliu
Copy link
Contributor

@eric-maynard from the 3 commits listed in the description (c3d50e1, e667670, and 5d5e0a3), i see only test files in the spark/v4.0 folder

do you know where i can find the reader being backported?

@eric-maynard
Copy link
Contributor Author

Hey @kevinjqliu, my mistake, this is really only porting the tests. So you're right that it should not be a blocker.

Copy link
Contributor

@stevenzwu stevenzwu left a comment

Choose a reason for hiding this comment

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

LGTM. just a nit comment

@stevenzwu stevenzwu merged commit b7ee7c0 into apache:main Sep 17, 2025
43 checks passed
@stevenzwu
Copy link
Contributor

Thanks @eric-maynard for the contribution, @nastra @huaxingao @kevinjqliu for the review

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.

5 participants