andygrove opened a new issue, #26296: URL: https://github.com/apache/datafusion/issues/26296
### Describe the bug In the `(DataType::Struct(lhs), DataType::Struct(rhs))` arm of `type_union_resolution_coercion` ([type_coercion/binary.rs#L835-L872](https://github.com/apache/datafusion/blob/0d79252f61bd8777990b2749c216ffd07079c30d/datafusion/expr-common/src/type_coercion/binary.rs#L835-L872)), each field's coerced **type** is computed by matching fields by name (`search_corresponding_coerced_type`), but the field's **nullability** is computed by zipping `lhs` and `rhs` positionally: ```rust let coerced_types = lhs .iter() .map(|lhs_field| search_corresponding_coerced_type(lhs_field, rhs)) .collect::<Option<Vec<_>>>()?; // preserve the field name and nullability let orig_fields = std::iter::zip(lhs.iter(), rhs.iter()); let fields: Vec<FieldRef> = coerced_types .into_iter() .zip(orig_fields) .map(|(datatype, (lhs, rhs))| coerce_fields(datatype, lhs, rhs)) .collect(); ``` `coerce_fields` sets `nullable = lhs.is_nullable() || rhs.is_nullable()`. When the two structs have the same field names in a different order, the nullability of each output field comes from an unrelated right-hand field. ### To Reproduce Resolve the union type of: - `lhs = Struct { a: Int32 NOT NULL, b: Int32 NOT NULL }` - `rhs = Struct { b: Int32 NULL, a: Int32 NOT NULL }` The result is `Struct { a: Int32 NULL, b: Int32 NOT NULL }`. Field `b` is reported as non-nullable even though the right-hand `b` can contain nulls (and `a` is needlessly marked nullable). ### Expected behavior Nullability is taken from the name-matched pair, giving `Struct { a: Int32 NOT NULL, b: Int32 NULL }`. For example, have `search_corresponding_coerced_type` return the matched `rhs_field` along with the coerced type, and pass that field to `coerce_fields`. ### Additional context The nearby `try_type_union_resolution_with_struct` (about L900-L970) walks `data_types` three times, each re-matching `DataType::Struct` with an `internal_err!` fallback the first loop already rules out. It may be worth simplifying in the same change. Line references are pinned to commit 0d79252f61bd. -- 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]
