-
Notifications
You must be signed in to change notification settings - Fork 7
[SPARK-52373] Add CRC32 struct
#191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| @Test | ||
| func testLongChecksum() async throws { | ||
| let str = String(repeating: "Apache Spark Connect Client for Swift", count: 1000) | ||
| #expect(CRC32.checksum(string: str, encoding: .ascii) == 1_985_943_888) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is consistent with JDK CRC32.
jshell> var c = new java.util.zip.CRC32();
jshell> c.update("Apache Spark Connect Client for Swift".repeat(1000).getBytes())
jshell> c.getValue()
$9 ==> 1985943888| @Test | ||
| func testChecksum() async throws { | ||
| let str = "Apache Spark Connect Client for Swift" | ||
| #expect(CRC32.checksum(string: str, encoding: .ascii) == 2_736_908_745) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is consistent with JDK CRC32.
jshell> var c = new java.util.zip.CRC32();
jshell> c.update("Apache Spark Connect Client for Swift".getBytes())
jshell> c.getValue()
$3 ==> 2736908745|
cc @viirya and @peter-toth |
|
I'll merge this to move forward~ |
viirya
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't look into the crc32 code in details. But if it is verified manually, it should be okay
|
Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but it's a bit unusual that we need to reimplement the common CRC32 algorithm.
|
Thank you, @peter-toth . Yes, indeed. I expected |
What changes were proposed in this pull request?
This PR aims to add
CRC32struct.Why are the changes needed?
Apache Spark Connect requires
CRC32checksum to implement APIs likeaddArtifacts.Does this PR introduce any user-facing change?
No behavior change.
How was this patch tested?
Pass the CIs.
Was this patch authored or co-authored using generative AI tooling?
Generated-by:
Gemini 2.5 Flash. Ask to write ASF-licensed CRC32 Swift code.CRC32.swiftI verified it manually and wrote test suite,
CRC32Tests.swift.