voonhous commented on code in PR #19687:
URL: https://github.com/apache/hudi/pull/19687#discussion_r3877859273
##########
hudi-common/src/main/java/org/apache/hudi/common/util/SortUtils.java:
##########
@@ -21,14 +21,127 @@
import org.apache.hudi.common.avro.HoodieAvroUtils;
import org.apache.hudi.common.model.HoodieRecord;
import org.apache.hudi.common.schema.HoodieSchema;
+import org.apache.hudi.common.schema.HoodieSchemaField;
+import org.apache.hudi.common.schema.HoodieSchemaType;
import org.apache.hudi.common.util.collection.FlatLists;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.exception.HoodieException;
+import java.util.Locale;
+import java.util.Map;
import java.util.function.Function;
+import java.util.stream.Collectors;
/**
* Utility functions used by BULK_INSERT practitioners while sorting records.
*/
public class SortUtils {
+
+ /**
+ * Rejects sort columns whose type cannot serve as a sort key. Spark's
RowOrdering.isOrderable
+ * is false for both VARIANT and MAP, which is the binding constraint on the
row path. On the
+ * Avro path only MAP is outright uncomparable (GenericData.compare throws
"Can't compare
+ * maps!"); a variant's {metadata, value} record does compare, but by its
bytes, which is never
+ * a meaningful sort key. The walk recurses through records and array
elements just as
+ * isOrderable does, so a struct or an array that merely holds a variant or
a map at depth is
+ * rejected too, and the error names the nested member that made the column
unorderable. Without
+ * this check the failure surfaces deep in the write job (an
AnalysisException from the row
+ * partitioner, a ClassCastException from the record-based one) without
naming the column.
+ *
+ * <p>Matching is case-insensitive, mirroring Spark's column resolution.
Names absent from the
+ * schema (nested paths, meta columns on a data-only schema) are left for
the caller to handle.
+ *
+ * @param sortColumns the configured sort columns, may be null or empty
+ * @param schema schema of the data, with or without metadata fields
+ */
+ public static void validateSortableColumns(String[] sortColumns,
HoodieSchema schema) {
+ if (sortColumns == null || sortColumns.length == 0
+ || schema == null || schema.getType() != HoodieSchemaType.RECORD) {
+ return;
+ }
+ Map<String, HoodieSchemaField> fieldsByLowerName =
schema.getFields().stream()
+ .collect(Collectors.toMap(field ->
field.name().toLowerCase(Locale.ROOT), Function.identity(), (first, second) ->
first));
+ for (String sortColumn : sortColumns) {
+ String columnName = sortColumn.trim();
+ HoodieSchemaField field =
fieldsByLowerName.get(columnName.toLowerCase(Locale.ROOT));
+ if (field == null) {
+ continue;
Review Comment:
Moved. `SortUtils.resolveSortColumn` now carries the segment walk (records
only, exact match before case-insensitive, `-1` split so `s.` does not
collapse), `validateSortableColumns` resolves every column through it before
checking the leaf, and `run_clustering` calls the same resolver instead of its
private copy. So `hoodie.clustering.plan.strategy.sort.columns = 's.tags'` is
rejected by the execution strategy / partitioner constructors with `Sorting by
column 's.tags' of type MAP is not supported` rather than Spark's
AnalysisException. Pinned in `TestSortUtils` (`s.m`, `S.M`, and the
non-resolving `s.missing` / `m.key_value.value` / `v.metadata` / `s.` / `.s`
left to the caller) and with an inline-clustering leg in
`TestClusteringProcedure`. Fixed in 725f0269ac0d.
--
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]
