adriangb commented on issue #25450: URL: https://github.com/apache/datafusion/issues/25450#issuecomment-6020829449
Thanks for looking into this @joseph-isaacs. If we go down this route, should we go one step further and remove the function-side default and let each source decide alone? - The default for `TableProvider` / `FileSource` is "accept nothing". - Parquet accepts only `get_field` with literal keys. The literal-key guard moves from `getfield.rs` into Parquet, because Parquet is the source that can read one leaf of a struct. - Vortex accepts the functions it can answer from its layout, for example `octet_length`. No change to the UDF is necessary. This also fixes the memory-table case in https://github.com/apache/datafusion/issues/14540, because a memory table accepts nothing. The cost is for third-party providers. If a provider gets `get_field` push-down today, it loses it until it implements the hook. The plans stay correct but can become slower. We will need an entry in the upgrade guide, and maybe a small helper for the `get_field` case. But not sure this matters, afaik no other provider was doing anything optimized with this. Two open questions: - Who is responsible for correctness? Some expressions must not move below a filter. Examples are volatile functions (`random()`) and expressions that can fail on rows that the filter removes (`1 / x` under `WHERE x <> 0`). Do we keep a small function-side flag for "safe to move", or do we make this a documented rule for each source? What do you think? - What about custom UDFs? E.g. https://github.com/datafusion-contrib/datafusion-functions-json uses these hooks to declare it's functions as "should be pushed down into the scan" and we apply rewrite rules via `PhysicalExprAdapter` to read shredded columns if available. How would this be implemented under the changes you propose? The stability requirement from the issue stays the same: the rule runs until the plan stops changing, so the source must give the same answer each time the rule asks. -- 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]
