Skip to content

Carry partial RowFn validity with MaskValues - #9534

Merged
connortsui20 merged 3 commits into
developfrom
ct/row-fn-validity-invariants
Aug 21, 2026
Merged

connortsui20 merged 3 commits into
developfrom
ct/row-fn-validity-invariants

Conversation

@connortsui20

@connortsui20 connortsui20 commented Aug 21, 2026 •

Copy link
Copy Markdown
Member

Summary

Row batch execution resolves validity once, but the valid-row executors repeat the check that the materialized mask is partial. Array-backed validity stays lazy on optimistic paths because resolving it can execute and linearly scan the full mask for the uncommon all-valid and all-null cases.

Changes

Matches the materialized Mask once and passes MaskValues through valid-row visitors, owned execution, and sink execution. Marks the all-null dense path as unreachable and documents the cost boundary on Validity::execute_mask.

@codspeed

codspeed Bot commented Aug 21, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 1981 untouched benchmarks
⏩ 54 skipped benchmarks1


Comparing ct/row-fn-validity-invariants (ccdccb9) with develop (c269954)

Open in CodSpeed

Footnotes

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

Comment on lines +65 to 74
// Do not resolve array-backed validity for the uncommon all-valid or all-null cases here.
// That can execute and scan the full mask; each policy resolves it only when necessary.
if self.validity.definitely_no_nulls() {
return self.execute_dense(kernel, ctx);
}

match self.policy {
RowPolicy::Dense => self.execute_dense(kernel, ctx),
RowPolicy::ValidOnly => self.execute_valid_only(kernel, try_valid_rows, ctx),
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

im realizing it is better to just match on the self.policy first and then only check for definitely_no_nulls in the valid-only case

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20
connortsui20 force-pushed the ct/row-fn-validity-invariants branch from 6e098c2 to ccdccb9 Compare August 21, 2026 16:28
@connortsui20
connortsui20 enabled auto-merge (squash) August 21, 2026 16:28
@connortsui20
connortsui20 merged commit 68cdf00 into develop Aug 21, 2026
91 of 94 checks passed
@connortsui20
connortsui20 deleted the ct/row-fn-validity-invariants branch August 21, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants