Skip to content

Add Validity::definitely_all_invalid and use it for AllInvalid fast paths#8335

Open
joseph-isaacs wants to merge 2 commits into
claude/cool-bardeen-l8jlsy-2-mask-eqfrom
claude/cool-bardeen-l8jlsy-3-definitely-all-invalid
Open

Add Validity::definitely_all_invalid and use it for AllInvalid fast paths#8335
joseph-isaacs wants to merge 2 commits into
claude/cool-bardeen-l8jlsy-2-mask-eqfrom
claude/cool-bardeen-l8jlsy-3-definitely-all-invalid

Conversation

@joseph-isaacs

Copy link
Copy Markdown
Contributor

Summary

PR 3 of a 4-PR stack (stacked on #8334) preparing Validity for lazy validity arrays.

Replaces the ~18 scattered matches!(.., Validity::AllInvalid) fast-path checks with a named helper, symmetric with definitely_no_nulls() from #8333. The name makes the conservative semantics explicit: a Validity::Array may still resolve to all-invalid once executed, so false means "unknown without compute", not "definitely has valid values".

Converted sites: the 8 DuckDB exporters, CUDA kernels (fsst, runend, datetime-parts, dynamic dispatch), list_contains, fill_null kernel, primitive top_value, dict codes, masked vtable, and varbin. The three assert!(matches!(...)) sites in tests keep the raw matches!, since they intentionally assert the exact variant.

No behavior change — purely a naming/readability refactor.

Checks

  • cargo nextest run -p vortex-array (2962 passed)
  • cargo nextest run -p vortex-duckdb (196 passed; test_geometry/over_http excluded — they need network access unavailable in this sandbox)
  • cargo clippy -p vortex-array -p vortex-duckdb --all-targets, cargo +nightly fmt --all
  • Could not build/test vortex-cuda locally (CUDA toolkit unavailable in this environment).

https://claude.ai/code/session_01VPQ7dfZtijfrsjAipwXvEj


Generated by Claude Code

…aths

Replace scattered matches!(.., Validity::AllInvalid) checks with a
named helper, symmetric with definitely_no_nulls(). The name makes the
conservative semantics explicit: a Validity::Array may still resolve to
all-invalid once executed, so a false result means "unknown without
compute", not "definitely has valid values".

Call sites that assert an exact variant in tests keep the raw matches!.

https://claude.ai/code/session_01VPQ7dfZtijfrsjAipwXvEj
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
The non-test code now uses definitely_all_invalid() and no longer
references the Validity type directly; the test module has its own
import.

https://claude.ai/code/session_01VPQ7dfZtijfrsjAipwXvEj
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
@codspeed-hq

codspeed-hq Bot commented Jun 10, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 15.07%

⚠️ 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.

⚡ 5 improved benchmarks
❌ 1 regressed benchmark
✅ 1520 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
WallTime cuda/bitpacked_u8/unpack/3bw[100M] 298.6 µs 349.5 µs -14.57%
Simulation chunked_bool_canonical_into[(1000, 10)] 47 µs 31.8 µs +47.79%
Simulation chunked_varbinview_canonical_into[(1000, 10)] 198.6 µs 161.8 µs +22.73%
Simulation chunked_varbinview_into_canonical[(1000, 10)] 213.2 µs 177.2 µs +20.3%
Simulation chunked_varbinview_canonical_into[(100, 100)] 308.9 µs 274.4 µs +12.57%
Simulation chunked_varbinview_into_canonical[(100, 100)] 362.1 µs 327.2 µs +10.66%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/cool-bardeen-l8jlsy-3-definitely-all-invalid (97b60ac) with claude/cool-bardeen-l8jlsy-2-mask-eq (4813bec)

Open in CodSpeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/chore A trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant