cshuo commented on code in PR #19502:
URL: https://github.com/apache/hudi/pull/19502#discussion_r3711966588
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/BaseSparkCommitActionExecutor.java:
##########
@@ -327,12 +327,11 @@ protected HoodieData<WriteStatus>
mapPartitionsAsRDD(HoodieData<HoodieRecord<T>>
if (table.requireSortedRecords()) {
// Partition and sort within each partition as a single step. This is
faster than partitioning first and then
// applying a sort.
- // requireSortedRecords() is true only for HFile base files, which order
keys by UTF-8 bytes,
- // not String (UTF-16) order, so sort with the matching comparator.
+ Comparator<String> recordKeyComparator =
HoodieRecordUtils.getRecordKeyComparator(table.getBaseFileFormat());
Comparator<Tuple2<HoodieKey, Option<HoodieRecordLocation>>> comparator =
(Comparator<Tuple2<HoodieKey, Option<HoodieRecordLocation>>> & Serializable)
(t1, t2) -> {
HoodieKey key1 = t1._1;
HoodieKey key2 = t2._1;
- return StringUtils.compareUtf8Bytes(key1.getRecordKey(),
key2.getRecordKey());
Review Comment:
Thanks for the detailed review. Two points:
1. Using UTF-8 consistently would be the ideal ordering contract, especially
for cross-engine compatibility. The current format-aware design is a
performance trade-off: HFile requires UTF-8 byte ordering, while UTF-8
comparison of Java String record keys is slower than String.compareTo,
particularly for long common prefixes. Details and benchmarks are in #19436.
If this sorting regression is acceptable, using UTF-8 ordering consistently
would be an optimal option.
2. Bulk insert and other LSM write paths are follow-up work. They are
tracked by #19437, #19438, #19439, and #19440. The relevant partitioners and
ordering requirements will be handled in those PRs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
