cetra3 commented on PR #25013: URL: https://github.com/apache/datafusion/pull/25013#issuecomment-6157852949
Thanks for this, and for the downstream validation in datafusion-variant. At Pydantic we're moving our attribute storage to shredded Variant, so this is directly on our path. I've been exploring the same problem, and opened #26282 as a draft. It builds on the ideas here, and I'd like to converge rather than run two competing designs. The main differences: 1. **One capability behind an accessor, instead of two hooks on `ScalarUDFImpl`.** `ScalarUDFImpl::as_nested_input_access()` returns `Option<&dyn NestedInputAccess>` and defaults to `None`. In this PR, `ScalarFunctionExpr::required_input_fields` resolves every argument's field (schema lookups, plus `reassign_expr_columns` and `return_field` for non-column arguments) before asking the UDF. It does that for every function node, at planning and again per file, even though almost every function returns `None`. With the accessor, functions without the capability cost one virtual call. It also keeps the scan-oriented contract off the main trait. 2. **`get_field` uses the same capability.** The Parquet read planner has a single code path for it and for any other function, with the same `get_field` behaviour as on `main`. That makes a separate `struct_field_access` unnecessary: a pure accessor implements `required_fields` the way `get_field` does. 3. **A second method, `output_statistics`, for row group and page pruning.** Given the statistics of the fields a call reads within a row group or page, the function returns statistics for its output. `get_field` passes the leaf's statistics through, so `WHERE s['a'] > 5` can now skip row groups and pages, which `main` can't do. A Variant accessor returns the typed leaf's statistics only where every fallback `value` on the path is null in that container (or the typed leaf has no nulls). That's the soundness guard Spark uses (SPARK-55817), and it keeps all Variant knowledge in the UDF. Inputs are still `ReturnFieldArgs` against the per-file physical schema, and requirements are still struct field paths, so the core idea is the same as yours. **Downstream validation.** I ported your datafusion-variant branch to it. `required_variant_input_fields` carried over almost unchanged. The one difference is that it lists leaves rather than ending a path at the typed subtree, because the draft only narrows to leaf paths so far. Your `parquet_input_fields` test passes unchanged against the new trait. I also added a test on real `shred_variant` data, with `k` shredded as Int32 and one value falling back. `variant_get(v, 'k', 'Int64') > 350` skips 2 of 4 row groups, and the row group holding the fallback value is always read and its row returned. **Benchmark.** The draft adds `benchmarks/src/bin/nested_access.rs`: 1M rows, 50 shredded keys, and a few fallback values in some row groups. Reading one key drops from 479 MB to 9.6 MB. A selective filter on a sorted key then drops from 19.6 MB to 6.4 MB with `output_statistics`, skipping 5 of 8 row groups. The two row groups with fallback values are correctly kept. How would you like to proceed? One option is reshaping this PR along those lines. Another is splitting #26282 into reviewable pieces with you as co-author: the trait plus read narrowing first, then statistics pruning. I'm happy either way, and I don't want to duplicate the work you've already done here. -- 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]
