adriangb commented on issue #19387:
URL: https://github.com/apache/datafusion/issues/19387#issuecomment-3675047904
> In the absence of Variant existing or backwards compatibility concerns, I
would possibly just suggest updating `Column` to something like:
>
> struct Column {
> name: String,
> index: usize,
> more_indexes: Vec<usize> // Just spitballing here
> }
>
> ...and `projection: Vec<usize>` to `projection: Projection` (where
`Projection` could be a `Vec<usize>` or maybe `Vec<Vec<usize>>`).
>
> In the presence of Variant existing, walking a `PhysicalExpr` for a
`get_field` ScalarUDF call is possibly much easier. Whatever registers variant
support could possibly replace `get_field()` with a ScalarUDF that can evaluate
against a variant array, too. (No opinions here, just spitballing).
I think this is what you are saying but my current thought is that given the
arbitrary extensibility we want from DataFusion it seems that creating a single
`PhysicalExpr` that has the "more indexes" info (be that adding a field to
`Column` or creating a new expression) is not going to be enough on it's own,
you also need information on how to extract that at runtime.
So my current thought is that allowing an arbitrary expression
(`get_field()`, `get_variant()`) to declare itself as a "column path
expression" seems like it will work better.
--
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]
