viirya commented on code in PR #57248:
URL: https://github.com/apache/spark/pull/57248#discussion_r3583914407
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/KeyGroupedPartitioningSuite.scala:
##########
@@ -4469,4 +4469,34 @@ class KeyGroupedPartitioningSuite extends
DistributionAndOrderingSuiteBase with
}
}
}
+
+ test("SPARK-58120: doCanonicalize preserves keyGroupedPartitioning
expression order") {
+ // Regression test: BatchScanExec.doCanonicalize previously used
+ // QueryPlan.normalizePredicates which combines expressions with And,
canonicalizes,
+ // then splits back. This reorders expressions, causing a mismatch between
+ // expression data types and partition key row values, leading to
ClassCastException.
+ val partition = Array(identity("id"), identity("data"))
+ createTable(table, columns, partition)
+ sql(s"INSERT INTO testcat.ns.$table VALUES " +
+ "(1, 'aa', cast('2020-01-01' as timestamp)), " +
+ "(2, 'bb', cast('2021-01-01' as timestamp)), " +
+ "(3, 'cc', cast('2022-01-01' as timestamp))")
+
+ val df = sql(s"SELECT id, data FROM testcat.ns.$table")
+ val scan = df.queryExecution.executedPlan.collect { case b: BatchScanExec
=> b }.head
+ val canonicalized = scan.canonicalized.asInstanceOf[BatchScanExec]
+
+ // The canonicalized keyGroupedPartitioning expressions must preserve the
same order
+ // as the original: [id (IntegerType), data (StringType)], not reversed.
+ val originalTypes = scan.keyGroupedPartitioning.get.map(_.dataType)
+ val canonicalizedTypes =
canonicalized.keyGroupedPartitioning.get.map(_.dataType)
+ assert(originalTypes == canonicalizedTypes,
Review Comment:
Thanks, this fully resolves it - and more thoroughly than I suggested. The
`QueryPlanSuite` test pins the reorder down at the right level: it exercises
`normalizePredicates` vs `normalizeExpressions` directly with fixed `ExprId`s,
so it doesn't depend on any end-to-end path happening to trigger the reorder,
and it can't go stale under a test-column refactor. I confirmed it locally -
the test passes on this branch, and since `orderCommutative` sorts by
`hashCode` over attributes with stable exprIds, the asserted `[StringType,
IntegerType]` order is deterministic rather than flaky. The cross-reference
comment you added in `KeyGroupedPartitioningSuite` is exactly the note I had in
mind. LGTM on this thread.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
