cblp commented on issue #26810:
URL: https://github.com/apache/datafusion/issues/26810#issuecomment-6663199697
I wanted to pick this up, but it looks like #26881 already changed the
behavior described here. `UNION` now merges field metadata with
`merge_metadata_for_union`, where the last input wins, and
`intersect_metadata_for_union` is deprecated and no longer called. On `main`
(sqllogictest, using its `with_metadata` UDF), the example from this issue
keeps the extension type:
```sql
SELECT arrow_metadata(x, 'ARROW:extension:metadata') FROM (
SELECT with_metadata(with_metadata(1, 'ARROW:extension:name',
'example.width'), 'ARROW:extension:metadata', '{"width":5}') AS x
UNION ALL
SELECT with_metadata(with_metadata(2, 'ARROW:extension:name',
'example.width'), 'ARROW:extension:metadata', '{ "width" : 5 }') AS x
) LIMIT 1;
-- { "width" : 5 }
```
So I think this issue can be closed. Do you agree?
The flip side is that inputs with different extension types are now merged
silently into the type of the last input:
```sql
-- the same, with '{"width":7}' in the second input
-- {"width":7}
SELECT arrow_metadata(x, 'ARROW:extension:name') FROM (
SELECT with_metadata(1, 'ARROW:extension:name', 'example.a') AS x
UNION ALL
SELECT with_metadata(2, 'ARROW:extension:name', 'example.b') AS x
) LIMIT 1;
-- example.b
```
Before #26881 the conflicting keys were dropped in these cases. Now the
values of the first input get the type of the last one. With GeoArrow, for
example, the rows of one input would be labeled with the CRS of another. For a
type that encodes values, such as an enum stored as codes with its labels in
the metadata, the rows would show the wrong labels. Postgres rejects such a
query (`UNION types mood and other cannot be matched`).
I would like to work on this as a follow-up and would open a separate issue
for it. Before I do, which direction would you prefer?
1. Return a planning error when the inputs of a `UNION` have different
`ARROW:extension:name` values, or the same name with different
`ARROW:extension:metadata`. To avoid rejecting the case from this issue,
compare the metadata as parsed JSON when both values parse as JSON, and as
strings otherwise. This needs no new API. A type whose metadata is not JSON, or
whose equivalent metadata differ beyond JSON formatting (e.g. equivalent CRS
definitions), would get an error instead of being merged. This is stricter than
today.
2. Let the extension type decide. Add a method to `DFExtensionType` that
checks whether two instances are the same type (or returns the common type),
and look the types up in the extension type registry. The problem is that the
`UNION` schema is merged without a registry, in `Union` (`plan.rs`),
`coerce_union_schema` and the physical `UnionExec`, so the registry would have
to be passed there. That is a larger change.
3. Do 1 now, and add the `DFExtensionType` hook from 2 later. The hook would
then refine the equality check.
I lean towards 3, but I'm happy to follow your preference.
--
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]
