Repository navigation
Move remaining expressions out of vortex-array to vortex-expr - #10768
robert3005 wants to merge 4 commits into
Conversation
d195163 to
b691d5a
Compare
Merging this PR will degrade performance by 9.1%2 warnings, 1 note
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
06bffcd to
137c718
Compare
b691d5a to
e57d7c2
Compare
137c718 to
0713434
Compare
e57d7c2 to
d8a74d2
Compare
0713434 to
e33c436
Compare
d8a74d2 to
123df13
Compare
595165d to
7644dd2
Compare
123df13 to
49d5a26
Compare
7644dd2 to
6814544
Compare
49d5a26 to
2e0ca58
Compare
6814544 to
1173f1a
Compare
2e0ca58 to
7ffa022
Compare
1173f1a to
dcaa482
Compare
ba6a3ba to
7c10fd1
Compare
dcaa482 to
9383bb2
Compare
7c10fd1 to
2d59169
Compare
| } | ||
|
|
||
| /// Bounds `lower(input)` from the byte-ordered min/max of `input`. | ||
| fn stat_expression( |
There was a problem hiding this comment.
this is funky, wonder if there's a nicer way
9383bb2 to
29fe7c3
Compare
Expressions are about to move out of vortex-array into vortex-expr, which means vortex-functions (whose scalar functions construct expressions) has to sit above vortex-expr rather than below it. This commit flips the dependency without moving the expression types yet: - `select` returns to vortex-array's core builtins. The field-path analyses in vortex-expr (`referenced_field_paths`, immediate access) narrow through it alongside `get_item`, so it is part of the projection machinery those analyses are built on, like `get_item` and `pack`. - `StatFn` moves from vortex-functions into `vortex_expr::stats::stat_fn`, registered by the new `vortex_expr::initialize`. - The stats rewrite rules for `between`, `is_nan`, `like`, `list_contains`, `dynamic` and `dynamic_expression` move into `vortex_functions::pruning`, registered by `vortex_functions::initialize`. The shared pieces they build on (stat references, NaN guards, literal and cast stat folding) become the public `vortex_expr::stats::rewrite::helpers` module. - `find_between` moves next to the `between` function it targets. Co-Authored-By: Claude Fable 5.1 <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Ps4mp7KF5c4p66aoSoXx5j Signed-off-by: Claude <[email protected]>
Unbound `Expression`, binding, traversal, lambdas, proto serialization and the expression constructors now live in `vortex-expr`; `vortex-array` keeps arrays, the scalar function machinery, the builtins it is built on, and `BoundExpression` with its reduce node and optimizer. The scalar function vtable reduces over bound expressions, so keeping them next to it lets every reduction rule stay monomorphized for expression trees as well as array trees. `vortex-expr` re-exports `BoundExpression`. The layering is `vortex-array -> vortex-expr -> vortex-functions`. - `ScalarFnVTable::simplify` is generic over `ReduceNode`, like `reduce`, but stays a separate hook that only the expression optimizer applies: a simplification may rewrite a node into one whose nullability differs, which the array optimizer cannot accept (it must keep the exact dtype and defer constant masks to the kernel rules). `ReduceNode::is_same` lets rules such as the COALESCE rewrite compare operands. - `new_expr`, `try_new_expr` and `try_new_bound_expr` move to `vortex_expr::ScalarFnExprExt`, `ArrayRef::apply`/`apply_bound` to `vortex_expr::ArrayExprExt`, and the expression half of the builtins to `vortex_expr::ExprBuiltins`. `ExprDisplay` moves to `vortex_array::scalar_fn::display`, and the tree renderer to `vortex_array::expr::display`. - `ArrayRef::take_slot_unchecked`/`put_slot_unchecked` become public, like `with_slots`, so row-count substitution in `vortex-expr` still rewrites lazy arrays in place. `vortex_array::expr` otherwise keeps only `stats`, which moves with the aggregate split. The core builtins' expression-level tests move to `vortex-expr`; the few array-level tests that used expressions now go through `ArrayBuiltins`. The `expr_*` benches move to `vortex-expr`, and their CodSpeed counts and durations move with them. Co-Authored-By: Claude Fable 5.1 <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Ps4mp7KF5c4p66aoSoXx5j Signed-off-by: Claude <[email protected]>
`select` and `is_null` are not needed by vortex-array's own arrays, so they move to vortex-functions with their constructors, builder methods (`ArrayFunctions::is_null`, `ExprFunctions::is_null`), tests and pruning rewrites (`vortex_functions::pruning`). `not` stays in vortex-array, because validity inversion and the `sum_v2` aggregate negate arrays lazily. - `get_item` read through `select` by naming it. It now asks its child through the new `ScalarFnVTable::forwarded_field` hook, which reports the child whose field an output field passes through unchanged. `select` implements it, so `get_item(select(x))` still reduces to `get_item(x)` for arrays and expressions. - `IsNull` and `IsNotNull` share the and/or null-check reduction through the public `reduce_null_check`; each passes its own nested check. - The field-access analyses `referenced_field_paths` and `make_bound_free_field_annotator`, which look through `select`, move from vortex-expr to `vortex_layout::analysis`, next to the scans that use them. Signed-off-by: Claude <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Ps4mp7KF5c4p66aoSoXx5j
…ex_array::stats `Stat`, `Precision`, the stat bounds and the `StatsProvider` traits describe the statistics every array carries, not expressions, so they join the per-array statistics cache (`ArrayStats`, `StatsSet`) in `vortex_array::stats`. `vortex_array::expr` now holds only `BoundExpression` and its optimizer. Callers move from `vortex_array::expr::stats` to `vortex_array::stats`. The umbrella crate's `vortex::expr::stats` keeps re-exporting the statistics vocabulary alongside vortex-expr's stat expressions. Signed-off-by: Claude <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Ps4mp7KF5c4p66aoSoXx5j
2d59169 to
10443fb
Compare
after #10762 we can further split expressions into vortex-expr. We flip dependency order into vortex-array → vortex-expr → vortex-functions.