sunchao commented on code in PR #6428:
URL: https://github.com/apache/datafusion-comet/pull/6428#discussion_r4178947089
##########
native/core/src/execution/planner.rs:
##########
@@ -1291,6 +1364,20 @@ impl PhysicalPlanner {
if !nested || lt.equals_datatype(&rt) {
return (left, right);
}
+ // Catalyst compares struct values by ordinal, even when their field
names differ.
+ // A name-based Arrow cast would change the values being compared.
+ if let Some(target) = Self::positional_nullability_union(<, &rt) {
Review Comment:
[P2] Could the early-return guard above use exact type equality (`lt == rt`)
so this reconciliation also handles different field names with identical
nullability? With native Range execution enabled on Spark 4.x, `SELECT
named_struct('x', CAST(id AS DOUBLE), 'y', 1D) = named_struct('y', 1D, 'x',
CAST(id AS DOUBLE)) FROM range(1, 2)` should return `true`. Preserving Catalyst
nullability now makes every field non-nullable, so `lt.equals_datatype(&rt)`
returns true despite the swapped names and bypasses this new code.
`spark_comparison` then fails with `Nested predicate requires matching types`.
The base succeeds for this input, so this change turns an executable query into
a failed task. Please retain field names in the guard and cover
equal-nullability operands.
Evidence: A disposable Rust test used the exact head constructor and
verbatim planner helpers, compared with the supplied base constructor and
reconciliation function. For id=1, the base returned BooleanArray[true], while
the head rejected Struct(x: non-null Float64, y: non-null Float64) versus
Struct(y: non-null Float64, x: non-null Float64). Changing only the guard to
`lt == rt` returned [true]. Spark 4.1.3 Catalyst accepted the expression,
inserted no struct-alignment cast, and evaluated it to true. Arrow 59.3.0
explicitly documents that `equals_datatype` ignores nested field names.
--
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]
