voonhous commented on code in PR #19687:
URL: https://github.com/apache/hudi/pull/19687#discussion_r3871027596
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/procedures/RunClusteringProcedure.scala:
##########
@@ -227,19 +246,38 @@ class RunClusteringProcedure extends BaseProcedure
prunedPartitions.map(partitionPath =>
partitionPath.getPath).toSet.mkString(",")
}
+ /**
+ * Validates the already-normalised (comma-separated, trimmed) order column
list against the
+ * table schema with its metadata fields, which is what the partitioners
sort on at execution
+ * time (the execution strategy adds them to the schema it hands the
partitioners), so a
+ * `_hoodie_*` column is accepted. A dotted path names a nested field: the
partitioners resolve
+ * it (getNestedFieldVal on the record path, Column(name) on the row path)
but the shared
+ * check only walks top-level names, so it is resolved here and its leaf
checked under the path.
+ */
private def validateOrderColumns(orderColumns: String, metaClient:
HoodieTableMetaClient): Unit = {
if (orderColumns == null) {
throw new HoodieClusteringException("Order columns is null")
}
val tableSchemaResolver = new TableSchemaResolver(metaClient)
- val fields = tableSchemaResolver.getTableSchema(false)
- .getFields.asScala.map(_.name().toLowerCase)
- orderColumns.split(",").foreach(col => {
+ val tableSchema = tableSchemaResolver.getTableSchema(true)
+ val fields = tableSchema.getFields.asScala.map(_.name().toLowerCase)
+ val (nestedColumns, topLevelColumns) =
orderColumns.split(",").partition(_.contains("."))
+ topLevelColumns.foreach(col => {
if (!fields.contains(col.toLowerCase)) {
throw new HoodieClusteringException("Order column not exist:" + col)
}
})
+ // The same validation the partitioners apply at execution time (see
+ // SortUtils.validateSortableColumns), surfaced here before the job is
submitted.
+ SortUtils.validateSortableColumns(topLevelColumns, tableSchema)
+ nestedColumns.foreach { col =>
+ val leaf = tableSchema.getNestedField(col)
Review Comment:
Done in 84bea41: `validateOrderColumns` resolves every column, top-level or
dotted, with its own segment walk -- exact match first, then case-insensitive,
descending only into records -- and checks the leaf with
`SortUtils.validateSortableColumn`; `getNestedField` is no longer used there.
`S.level` resolves and the clustering sorted on it completes,
`s.tags.key_value.value` is reported as not existing, and `s.tags` is still
rejected as a MAP; all three in the `run_clustering` sort-column test.
--
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]
