james-willis commented on code in PR #24865:
URL: https://github.com/apache/datafusion/pull/24865#discussion_r4404762602
##########
datafusion/common/src/metadata.rs:
##########
@@ -117,6 +118,75 @@ pub fn check_metadata_with_storage_equal(
Ok(())
}
+/// Compute the field metadata to propagate for an expression whose result
+/// may come from any one of several inputs coerced to a common type, such as
+/// `COALESCE`, `NVL2`, or `CASE WHEN`.
+///
+/// Metadata is merged across the fields whose data type is not `Null` (an
+/// untyped NULL literal carries no metadata and cannot contribute a typed
+/// value), with earlier fields taking precedence for repeated keys. Unrelated
+/// keys are not required to agree: metadata that arrived with the data (e.g.
+/// from an Arrow file or an embedded Arrow schema in a Parquet file) must not
+/// cause an extension type to be silently dropped from the result.
+///
+/// The Arrow extension type keys are handled as a unit, like in
+/// [`Expr::Cast`] output field derivation:
+/// - Differing values of `ARROW:extension:name` produce a planning error;
+/// the inputs are of different logical types even though they were coerced
+/// to a common storage type.
+/// - `ARROW:extension:metadata` is taken from the field that supplied the
+/// extension name, never combined from another field. Byte-wise differences
+/// in the extension metadata of fields that agree on the extension name are
+/// tolerated, since extension parameters are often JSON-encoded, where byte
+/// equality is too strict.
+///
+/// [`Expr::Cast`]:
https://docs.rs/datafusion/latest/datafusion/logical_expr/enum.Expr.html#variant.Cast
+pub fn coerced_fields_metadata<'a>(
Review Comment:
I don't totally understand the `UNION` plumbing but
`intersect_metadata_for_union` is only used for the first stage and is not what
`UNION` will ultimately give for the query result's type. The ultimate behavior
of UNION is also not great here.
I'm inclined to keep this PR as-is because I think probably the different
branches need to agree at least on the extension type and it is important to
enforce that. Even better we do it at plan time.
I would like to be more strict with the other metadata because often that is
part of the type (e.g., the CRS is part of the type of a geometry, randomly
picking one of the CRSes will give incorrect query results) but I'm not
confident in that being universally the right call to enforce it here (e.g., in
the CRS case we would want to convert to item-level CRS when branches diverge).
--
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]
