viirya commented on code in PR #6760:
URL: https://github.com/apache/datafusion-comet/pull/6760#discussion_r4232640966
##########
native/spark-expr/src/conditional_funcs/case_when.rs:
##########
@@ -162,6 +163,38 @@ fn coerce_branch(
))
}
+/// Casts the arguments of a Spark `ComplexTypeMergingExpression` such as
`greatest` or `least` to
+/// their common type, which keeps the first argument's field names, as
Spark's result type does.
+///
+/// Spark has already given the arguments the same SQL type up to nullability,
and up to the case
+/// of struct field names when the analysis is case-insensitive. It compares
structs field by field
+/// by position. DataFusion's struct coercion and Arrow's struct cast both
match fields by name
+/// when two structs hold the same set of names, which pairs different
positions when the names
+/// differ only in case, so the arguments are reconciled here positionally
instead. The arguments
+/// are returned unchanged when they have no positional common type.
+pub fn coerce_to_common_type(
Review Comment:
Agreed. #6428 is still open, so this PR keeps its own helper for now.
Whichever lands second will fold `positional_common_type(..,
PositionalTypeCoercion::MetadataOnly)` over the arguments and cast each one
with `cast_to_common_type`, dropping `coerce_to_common_type`. That leaves one
path for `IN`, `greatest` and `least`. If #6428 lands first, I'll rebase this
PR onto it and make that change here.
##########
native/spark-expr/src/conditional_funcs/case_when.rs:
##########
@@ -1577,4 +1610,95 @@ mod tests {
Some(null)
));
}
+
+ /// `greatest` and `least` arguments whose struct fields hold the same
names in another order,
+ /// which Spark accepts when the names differ only in case. They are cast
to the first
+ /// argument's type by position, with each field nullable if any
argument's is. Matching the
+ /// fields by name would put the second argument's `x` value in the first
position.
+ #[test]
+ fn coerce_to_common_type_is_positional() {
Review Comment:
Dropped in e1d88f287.
##########
spark/src/test/resources/sql-tests/expressions/math/greatest_least_struct_field_case.sql:
##########
@@ -0,0 +1,83 @@
+-- Licensed to the Apache Software Foundation (ASF) under one
+-- or more contributor license agreements. See the NOTICE file
+-- distributed with this work for additional information
+-- regarding copyright ownership. The ASF licenses this file
+-- to you under the Apache License, Version 2.0 (the
+-- "License"); you may not use this file except in compliance
+-- with the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing,
+-- software distributed under the License is distributed on an
+-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+-- KIND, either express or implied. See the License for the
+-- specific language governing permissions and limitations
+-- under the License.
+
+-- With case-insensitive analysis, Spark accepts greatest and least arguments
whose struct field
+-- names differ only in case, and compares the structs field by field by
position. The result
+-- takes the first argument's field names. Here the second argument holds the
same names in the
+-- other order, so matching the fields by name instead would pair x with x and
give a different
+-- row for id = 0 and id = 2.
+
+-- Config: spark.comet.exec.range.enabled=true
+-- Config: spark.comet.sparkToColumnar.enabled=true
+-- Config: spark.comet.sparkToColumnar.supportedOperatorList=Range
Review Comment:
Dropped both lines in e1d88f287. Every query in the file still runs natively
through `CometRangeExec`.
--
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]
