parthchandra commented on PR #6458:
URL:
https://github.com/apache/datafusion-comet/pull/6458#issuecomment-5940740984
The positional struct-field reconciliation here looks right, and it now
makes IF more correct than CASE WHEN / COALESCE. Those two still match struct
fields by name: `create_case_when`
(`native/spark-expr/src/conditional_funcs/case_when.rs:48`, used by both CASE
and COALESCE) computes its common type via DataFusion's
`get_coerce_type_for_case_expression`, which folds with `type_union_coercion` -
the same name-based matcher you just replaced with the positional
`if_common_type` for IF.
So a case-variant struct whose field names are swapped by position, e.g.
`CASE WHEN q THEN named_struct('x', i, 'X', dbl) ELSE named_struct('X', int,
'x', dbl) END`, would mis-coerce in Comet while Spark aligns positionally
(Spark's `findTypeForComplex` zips fields by index and takes the left branch's
name, `TypeCoercionHelper.scala:238-251`). The one CASE WHEN test here
(`if_nested_nullability.sql:113`) only uses a same-name single-field struct, so
it doesn't catch this.
Pre-existing and out of scope for this PR, but since this PR leaves IF ahead
of CASE/COALESCE, could you file a tracking issue to extend the positional
helper to the CASE/COALESCE path?
--
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]
