Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Dec 4, 2025

What changes were proposed in this pull request?

This PR aims to upgrade lz4-java to 1.10.0 and exclude the legacy groupID version.

Why are the changes needed?

Since lz4-java changed its repository, we had better depend on the live repository for future maintenance.

Does this PR introduce any user-facing change?

No Spark behavior change.

How was this patch tested?

Pass the CIs.

Was this patch authored or co-authored using generative AI tooling?

No.

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Dec 4, 2025

This is a dependency-only PR, cc @dbtsai , @HyukjinKwon , @LuciferYang , @yawkat , @SteNicholas .

To be clear, the security issue is not a scope of this PR.

@dongjoon-hyun
Copy link
Member Author

Thank you, @HyukjinKwon . I'm going to add one more commit to ban this library explicitly.

@dongjoon-hyun
Copy link
Member Author

All tests passed at the first commit and the subsequent two comments are only about excluding the transitive dependencies to make it sure.

Screenshot 2025-12-04 at 5 23 23 PM

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM

@SteNicholas
Copy link
Member

SteNicholas commented Dec 5, 2025

@dongjoon-hyun, does it still need to switch fastDecompressor to safeDecompressor after upgrade?

@LuciferYang
Copy link
Contributor

LuciferYang commented Dec 5, 2025

org.lz4:lz4-java

spark/NOTICE-binary

Lines 205 to 208 in eaaa7ff

* LICENSE:
* license/LICENSE.lz4.txt (Apache License 2.0)
* HOMEPAGE:
* https://github.com/jpountz/lz4-java

Should the LICENSE-binary and NOTICE-binary be updated?

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Dec 5, 2025

@dongjoon-hyun, does it still need to switch fastDecompressor to safeDecompressor after upgrade?

Exactly, that's @dbtsai 's contribution, @SteNicholas . This PR doesn't aim to do that. He will rebase his PR after merging this independently.

@dongjoon-hyun
Copy link
Member Author

Thank you, @LuciferYang ! I'll update it.

@SteNicholas
Copy link
Member

@dongjoon-hyun, I just confirm whether to switch fastDecompressor to safeDecompressor after upgrade to 1.10.0.

@dongjoon-hyun
Copy link
Member Author

I just confirm whether to switch fastDecompressor to safeDecompressor after upgrade to 1.10.0.

@SteNicholas What I can say here is that it's beyond of this PR. Technically, we don't know what decision we are going to make eventually on the following yet because it's still Draft.

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Dec 5, 2025

Please don't get me wrong. I'm trying to help that PR move forward by reducing the gap.

@LuciferYang
Copy link
Contributor

LGTM ~

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Dec 5, 2025

Thank you all! Merged to master for Apache Spark 4.2.0 (for now)

@dongjoon-hyun dongjoon-hyun deleted the SPARK-54597 branch December 5, 2025 03:02
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.

4 participants