Skip to content

RowFn framework and machinery - #9319

Closed
connortsui20 wants to merge 3 commits into
developfrom
ct/row-fn-api
Closed

connortsui20 wants to merge 3 commits into
developfrom
ct/row-fn-api

Conversation

@connortsui20

@connortsui20 connortsui20 commented Aug 10, 2026 •

Copy link
Copy Markdown
Member

Tracking Issues: #9129, #9130

Adds the RowFn API, its private batch executor, and focused executor benchmarks.

TODO

@connortsui20 connortsui20 added the changelog/feature A new feature label Aug 10, 2026
Introduce typed row execution with self-contained kernel arguments and explicit execution contracts. Validate decoded lengths once per batch while preserving specialized mixed-constant loops.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Cover RowFn executor overhead and strict validity policies with focused microbenchmarks.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@codspeed

codspeed Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Merging this PR will improve performance by 26.84%

⚡ 1 improved benchmark
✅ 1958 untouched benchmarks
🆕 16 new benchmarks
⏩ 89 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Simulation decode_varbin[(1000, 2)] 78.5 µs 61.9 µs +26.84%
🆕 Simulation handrolled_sink_wrapping_add N/A 1.7 ms N/A
🆕 Simulation row_checked_add N/A 2.1 ms N/A
🆕 Simulation row_checked_add_constant N/A 1.6 ms N/A
🆕 Simulation row_checked_add_nullable N/A 2.1 ms N/A
🆕 Simulation row_sink_wrapping_add N/A 2.8 ms N/A
🆕 Simulation row_wrapping_add N/A 1.8 ms N/A
🆕 Simulation row_wrapping_add_constant N/A 1.2 ms N/A
🆕 Simulation row_wrapping_add_nullable N/A 1.8 ms N/A
🆕 Simulation eager_chain[1048576] N/A 25.2 ms N/A
🆕 Simulation eager_chain[65536] N/A 1.9 ms N/A
🆕 Simulation eager[1048576] N/A 9.3 ms N/A
🆕 Simulation eager[65536] N/A 659.4 µs N/A
🆕 Simulation lazy_chain[1048576] N/A 20.5 ms N/A
🆕 Simulation lazy_chain[65536] N/A 1.3 ms N/A
🆕 Simulation lazy[1048576] N/A 9.2 ms N/A
🆕 Simulation lazy[65536] N/A 648.7 µs N/A

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ct/row-fn-api (f759998) with develop (ff0a26d)

Open in CodSpeed

Footnotes

  1. 89 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. ↩

@connortsui20
connortsui20 marked this pull request as ready for review August 10, 2026 02:58
@connortsui20 connortsui20 changed the title Add the RowFn scalar function framework RowFn API Aug 10, 2026
@connortsui20 connortsui20 changed the title RowFn API RowFn framework and machinery Aug 10, 2026
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>

@joseph-isaacs joseph-isaacs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll continue tomorrow.

i would be interested in keeping the internals private so we can iterate

#[derive(Clone, Copy)]
pub struct KernelArgs<'a> {
/// The input arrays for this kernel invocation.
pub arrays: &'a [ArrayRef],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this struct not exist?

}

/// One batch of inputs and the metadata needed before its row kernel runs.
pub struct Batch {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it need to be pub?

Dense,

/// Evaluate all rows, retrying only valid rows if a deferred error is raised.
DenseWithRetry,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we remove this initially?

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum RowPolicy {
/// Evaluate all rows and mask the result.
Dense,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another option is zipping output with rows and combing valid and fail?

pub trait RowFn: 'static + Sized + Clone + Send + Sync {
/// Options for this function, if any. Use [`EmptyOptions`](crate::scalar_fn::EmptyOptions)
/// for none.
type Options: 'static + Send + Sync + Clone + Debug + Display + PartialEq + Eq + Hash;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we give this a name and use it with scalar fn and here

/// - The output **must not** introduce a null where every input is valid.
///
/// The framework skips this hook for nullary functions.
fn reduce_encoded(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this used?

ArgColumnKind<T>,
);

enum ArgColumnKind<T: InputElement> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should use this throughout all of vortex

@connortsui20
connortsui20 marked this pull request as draft August 11, 2026 03:13
@connortsui20
connortsui20 deleted the ct/row-fn-api branch August 11, 2026 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants