Skip to content

fix(spark): make binary clustering/bulk-insert sort keys Comparable - #19622

Open
lokeshj1703 wants to merge 2 commits into
apache:masterfrom
lokeshj1703:eng-46937-binary-clustering-sort-keys
Open

fix(spark): make binary clustering/bulk-insert sort keys Comparable#19622
lokeshj1703 wants to merge 2 commits into
apache:masterfrom
lokeshj1703:eng-46937-binary-clustering-sort-keys

Conversation

@lokeshj1703

@lokeshj1703 lokeshj1703 commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Describe the issue this Pull Request addresses

Closes #19621

Summary and Changelog

Clustering and bulk-insert sort keys flow through SortUtils.getComparableSortColumns -> FlatLists.ofComparableArray, which casts every sort value to Comparable. On the Spark record path a BINARY sort column arrives as a raw byte[] (not Comparable), so ofComparableArray threw ClassCastException: [B cannot be cast to java.lang.Comparable, failing every clustering / bulk-insert on that column. The Avro path was unaffected because HoodieAvroUtils.getNestedFieldVal returns a java.nio.ByteBuffer for the same column.

This wraps byte[] into ByteBuffer.wrap(...) inside the shared HoodieUTF8StringFactory.wrapArrayOfObjects hook used by both RDDCustomColumnsSortPartitioner and RDDBucketIndexPartitioner. ByteBuffer is Comparable, and it survives the sortBy shuffle because the sort key FlatLists.ComparableList is KryoSerializable and Hudi always runs the write path with Kryo (HoodieSparkSqlWriter rejects any other spark.serializer) -- so the wrapped byte[] rides Kryo exactly as the Avro binary path already does. (ByteBuffer is not java.io.Serializable, and neither is the key; the shuffle uses Kryo, not Java serialization.) It preserves the exact byte-lexicographic ordering the Avro path already produced, so the change is behavior-preserving and needs no change to FlatLists. Adds two tests to TestBulkInsertInternalPartitioner: testSortColumnsWithBinaryValueAreComparable reproduces the CCE without the fix and asserts the ordering with it, and testBinarySortKeySurvivesKryoRoundTrip round-trips the wrapped key through Kryo (via SerializationUtils) and asserts the element stays a ByteBuffer with its ordering preserved (verified to fail without the fix with the same ClassCastException).

Impact

Clustering and bulk_insert can sort by BINARY columns on the Spark record path. No behavior change for existing (string / primitive) sort columns.

Risk Level

low

Behavior-preserving: the byte-lexicographic ordering is identical to the pre-existing Avro path, and it is covered by a new unit test that fails without the change. The wrapped ByteBuffer sort key is shuffled by the sort partitioner exactly as the Avro binary path already shuffles it (the same ByteBuffer in the same FlatLists.ComparableList), so there is no new serialization behavior (the key rides Kryo, same as the Avro path).

Documentation Update

none

Contributor's checklist

  • Read through contributor's guide
  • Enough context is provided in the sections above
  • Adequate tests were added if applicable

Clustering and bulk-insert sort keys flow through
SortUtils.getComparableSortColumns -> FlatLists.ofComparableArray, which
casts every sort value to Comparable. On the Spark record path a binary
(BINARY/bytes) sort column arrives as a raw byte[], which is not
Comparable, so ofComparableArray threw:

  java.lang.ClassCastException: [B cannot be cast to java.lang.Comparable
      at FlatLists.ofComparableArray(FlatLists.java:55)

The Avro path never hit this because getNestedFieldVal yields a
java.nio.ByteBuffer for the same column (and BaseSparkInternalRecordContext
already wraps byte[] as ByteBuffer elsewhere). Wrap byte[] into
ByteBuffer.wrap(...) inside the shared HoodieUTF8StringFactory hook that
both RDDCustomColumnsSortPartitioner and RDDBucketIndexPartitioner use.
ByteBuffer is Comparable + Serializable (survives the sortBy shuffle) and
preserves the exact byte-lexicographic ordering the Avro path produced, so
this is behavior-preserving with no FlatLists change.

Adds TestBulkInsertInternalPartitioner#testSortColumnsWithBinaryValueAreComparable,
which reproduces the CCE without the fix and asserts correct ordering with it.
@lokeshj1703
lokeshj1703 force-pushed the eng-46937-binary-clustering-sort-keys branch from aa4e6e0 to 4b1cbed Compare August 13, 2026 11:59
@github-actions github-actions Bot added the size:S PR with lines of changes in (10, 100] label Aug 13, 2026
@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.80%. Comparing base (4853b57) to head (580067d).
⚠️ Report is 45 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19622      +/-   ##
============================================
+ Coverage     77.09%   77.80%   +0.70%     
- Complexity    32490    33059     +569     
============================================
  Files          2522     2525       +3     
  Lines        139112   139645     +533     
  Branches      16714    17075     +361     
============================================
+ Hits         107243   108645    +1402     
+ Misses        24291    23382     -909     
- Partials       7578     7618      +40     
Components Coverage Δ
hudi-common 83.31% <ø> (+0.05%) ⬆️
hudi-client 82.77% <100.00%> (+0.05%) ⬆️
hudi-flink 85.74% <ø> (+0.39%) ⬆️
hudi-spark-datasource 72.26% <ø> (+1.65%) ⬆️
hudi-utilities 73.85% <ø> (+0.22%) ⬆️
hudi-cli 15.26% <ø> (-0.07%) ⬇️
hudi-hadoop 69.03% <ø> (+5.53%) ⬆️
hudi-sync 75.36% <ø> (+0.24%) ⬆️
hudi-io 79.42% <ø> (-0.04%) ⬇️
hudi-timeline-service 83.44% <ø> (ø)
hudi-cloud 64.33% <ø> (+0.26%) ⬆️
hudi-kafka-connect 53.20% <ø> (ø)
Flag Coverage Δ
common-and-other-modules 50.93% <0.00%> (+0.19%) ⬆️
flink-integration-tests 49.13% <ø> (+0.04%) ⬆️
hadoop-mr-java-client 43.84% <ø> (+0.10%) ⬆️
integration-tests 13.62% <0.00%> (+0.05%) ⬆️
spark-client-hadoop-common 50.57% <0.00%> (+0.95%) ⬆️
spark-java-tests 51.88% <100.00%> (+0.27%) ⬆️
spark-scala-tests 46.46% <0.00%> (+0.49%) ⬆️
utilities 36.59% <50.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...org/apache/spark/sql/HoodieUTF8StringFactory.scala 100.00% <100.00%> (ø)

... and 133 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lokeshj1703
lokeshj1703 marked this pull request as ready for review August 13, 2026 12:45

@hudi-agent hudi-agent left a comment

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.

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for working on this! This PR fixes a ClassCastException when clustering or bulk-inserting with a BINARY sort column on the Spark record path, by wrapping raw byte[] into a ByteBuffer in the shared wrapArrayOfObjects hook so it becomes Comparable with the same byte-lexicographic ordering the Avro path already produces. I traced the sort-key path (SortUtilsFlatLists.ofComparableArray) and the serialization path (ComparableListImpl is KryoSerializable, matching how the Avro ByteBuffer keys already shuffle), and the ordering and edge-case behavior line up with the existing Avro path. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.

cc @yihua

@hudi-agent hudi-agent left a comment

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.

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for working on this! The PR fixes a ClassCastException on the Spark clustering/bulk-insert path when sorting by a BINARY column by wrapping raw byte[] into ByteBuffer.wrap(...) in the shared wrapArrayOfObjects hook, matching the ordering the Avro path already produces. I traced the reachable call paths (only the Spark wrapUTF8StringFunc overload hits this; the Java/Avro overload is unaffected), the aliasing safety of ByteBuffer.wrap (the byte[] is a per-record value from getBinary, not a reused buffer), and the signed byte-lexicographic ordering equivalence with the Avro ByteBuffer path — all consistent. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.

cc @yihua

@hudi-bot

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S PR with lines of changes in (10, 100]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clustering / bulk_insert on a BINARY (bytes) sort column throws ClassCastException: [B cannot be cast to java.lang.Comparable

4 participants