Simplify RowFn execution contracts - #9496
Conversation
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
| @@ -74,7 +74,7 @@ impl<F: RowFn> ScalarFnVTable for F { | |||
| } | |||
|
|
|||
| fn is_fallible(&self, _options: &Self::Options) -> bool { | |||
There was a problem hiding this comment.
@joseph-isaacs should we change this to is_infallible
Merging this PR will degrade performance by 20.48%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | take_map[(0.1, 1.0)] |
245.6 µs | 325.7 µs | -24.57% |
| ❌ | Simulation | take_map[(0.1, 0.5)] |
161.5 µs | 192.6 µs | -16.17% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ct/row-fn-cleanup (aaf8e52) with develop (b81420e)
Footnotes
-
517 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
| let views = Args::views_if_no_consts(&columns); | ||
| if let Some(views) = views { | ||
| if Args::ARITY != 0 && views.len() != row_count { | ||
| if !Args::view_lens_match(&views, row_count) { |
There was a problem hiding this comment.
We would rather this return false and construct the error than for views.len to have to panic because one of the views doesn't have the same length as the others
| @@ -74,7 +74,7 @@ impl<F: RowFn> ScalarFnVTable for F { | |||
| } | |||
|
|
|||
| fn is_fallible(&self, _options: &Self::Options) -> bool { | |||
## Rationale for this change - Stacked on: #9496 - Tracking issue: #9130 - API tracking: #9129 - Epic: #9128 Connects the row execution layer to the scalar-function adapter. ## What changes are included in this PR? Adds constant handling, strict validity propagation, dense execution, direct valid-row execution for supporting sinks, and output validation. A partially valid signature that cannot execute directly on valid rows panics in this layer. Encoding-aware reductions remain in #9347. Filter-and-scatter remains in #9349. Owned valid-row execution and its `Default` bound are in #9500. Nullary execution is in #9469. ## What APIs are changed? Are there any user-facing changes? Adds execution internals within the existing `unstable_row_fns` boundary. There are no stable API changes. --------- Signed-off-by: Connor Tsui <connor.tsui20@gmail.com> Signed-off-by: Connor Tsui <connor@spiraldb.com>
2 participants
Rationale for this change
RowFnover Vortex arrays #9130RowFnAPI #9129Separates the mechanical
RowFncleanup from batch execution in #9450.What changes are included in this PR?
Renames the fallibility constants to the positive
INFALLIBLEcontract, removesRowExecutionandDenseWithRetry, and returns arrays directly from the executors. The tuple guard checks every decoded view length before unchecked row access becauseViewLen::lenreturns one member length after its debug assertion.What APIs are changed? Are there any user-facing changes?
Renames the unstable
RowFn::FALLIBLEandSinkResult::FALLIBLEassociated constants toINFALLIBLE. There are no stable API changes.