cshuo commented on code in PR #19727:
URL: https://github.com/apache/hudi/pull/19727#discussion_r3853514284
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/BucketSortBulkInsertPartitioner.java:
##########
@@ -32,13 +34,26 @@ public abstract class BucketSortBulkInsertPartitioner<T>
implements BulkInsertPa
public BucketSortBulkInsertPartitioner(HoodieTable table, String sortString)
{
this.table = table;
+ validateCustomSortColumns(table, sortString);
if (!StringUtils.isNullOrEmpty(sortString)) {
this.sortColumnNames = sortString.split(",");
} else {
this.sortColumnNames = null;
}
}
+ public static void validateCustomSortColumns(HoodieTable table, String
sortString) {
Review Comment:
Fixed in 44cf6615b831. Good point — this config is ignored by bucket-index
partitioners, as it was before LSM support, so validating it was redundant. The
validation now only checks the actual .
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/execution/bulkinsert/RDDBucketIndexPartitioner.java:
##########
@@ -121,7 +122,10 @@ private JavaRDD<HoodieRecord<T>>
doPartitionAndSortByRecordKey(JavaRDD<HoodieRec
LOG.warn("Bucket index does not support global sort mode, the sort will
only be done within each data partition");
}
- Comparator<HoodieKey> comparator = (Comparator<HoodieKey> & Serializable)
(t1, t2) -> t1.getRecordKey().compareTo(t2.getRecordKey());
+ boolean lsmTable =
table.getMetaClient().getTableConfig().isLSMTreeStorageLayout();
+ Comparator<HoodieKey> comparator = (Comparator<HoodieKey> & Serializable)
(t1, t2) -> lsmTable
Review Comment:
Fixed in 44cf6615b831. The comparator is now selected once based on the
storage layout before sorting.
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/commit/DatasetBucketRescaleCommitActionExecutor.java:
##########
@@ -57,7 +57,9 @@ public
DatasetBucketRescaleCommitActionExecutor(HoodieWriteConfig config,
*/
@Override
protected BulkInsertPartitioner<Dataset<Row>> getPartitioner(boolean
populateMetaFields, boolean isTablePartitioned) {
- return new
BucketIndexBulkInsertPartitionerWithRows(writeClient.getConfig(), expression,
rule, bucketNumber);
+ return new BucketIndexBulkInsertPartitionerWithRows(
Review Comment:
Addressed in 44cf6615b831. The config validation was removed because is
ignored by bucket-index partitioners. Therefore the rescale path no longer
bypasses a relevant validation or risks failing on this config after persisting
the hashing config.
--
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]
