Repository navigation
Reduce constant projections through select and extension storage - #11056
Conversation
Signed-off-by: Nicholas Gates <[email protected]>
joseph-isaacs
left a comment
There was a problem hiding this comment.
@connortsui20 this violates the semantics of ext correct?
Merging this PR will regress 2 benchmarks1 warning
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
|
This should be fine because there is already an explicit |
| fn reduce<T: ReduceNode>(&self, _options: &Self::Options, node: &T) -> VortexResult<Option<T>> { | ||
| Ok(node | ||
| .child(0) | ||
| .as_constant() | ||
| .map(|scalar| node.new_constant(scalar.as_extension().to_storage_scalar()))) | ||
| } |
There was a problem hiding this comment.
So I actually want to do this myself later, but generalized to things beyond constant. I would probably rename the method though (reduce_constant()?).
U2, U3, U4, U6, U7 and U8 merged on 2026-11-02 in #11012, #11014 and #11016, and the DataFusion literal fix #11067 merged right after U2. So order constraint 1 holds, and phase 1 starts from develop. The graph shows these nodes as merged, and parts 2 and 4 name the PR that fixed each finding. #10953 now keeps i32 days and widens only the days part to i64 when a value does not fit. It has no review, and it conflicts with the DateTimeParts compare kernel that U2 changed. #11056 still waits on its request for changes. #10661 replaces the closed #10482 for file sort statistics. In the type roadmap, only the N1 sentence changes. Signed-off-by: Connor Tsui <[email protected]>
Signed-off-by: Nicholas Gates <[email protected]>
|
Lints are failing |
Signed-off-by: Nicholas Gates <[email protected]>
SR merges into SD, because a storage declaration without the exact read check reads an old form silently wrong. SD now needs SN1, which only tightens the spatial validators. SN2 takes every Arrow importer of a known type and builds on DP, which peels dictionary fields before the importers run. V10 and DS become their own PRs. Also record that #10953 is pushed at 7943b4e and waits for an approval, that the 12.9% millisecond slowdown was measured at d6e73bd, and that #11056 merged. Signed-off-by: Connor Tsui <[email protected]>
Summary
Extracting a constant extension-valued field through
select, then extracting its storage, can miss the immediate-input constant execution paths and request canonical arrays. Reduce these operations during expression and array optimization so the result remains constant.For example, the regression test uses an input struct with a constant
measurementfield whose dtype is an extension over storage{value: i32}, alongside a varyingotherfield:Here the inner
get_itemreturns the extension-valuedmeasurementfield.ext_storageunwraps that value to its storage struct, and the outerget_itemextractsvalue.Changes
get_itemon constant structs andext_storageon constant extensions, sharing the existing field extraction logic with execution.The projection rewrite currently uses the shared scalar-function reducer. Moving array rewrites into the session kernel registry requires wiring session-aware optimization into the relevant callers and is left for follow-up.
Validation
cargo +nightly-2026-09-10 fmt -p vortex-array.cargo clippy -p vortex-array --all-targets --all-features -- -D warnings.