Remove duplicate ValidityVTable logic#5975
Merged
Conversation
Signed-off-by: Nicholas Gates <nick@nickgates.com>
gatesn
added
action/benchmark
github-actions
bot
removed
the
action/benchmark
Contributor
Benchmarks: TPC-H SF=1 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: TPC-H SF=10 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: Random AccessSummary
|
Contributor
Benchmarks: TPC-DS SF=1 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: FineWeb S3Summary
Detailed Results Table
|
Contributor
Benchmarks: TPC-H SF=1 on S3Summary
Detailed Results Table
|
Merging this PR will degrade performance by 35.79%
Performance Changes
Comparing 
Footnotes
|
Contributor
Benchmarks: TPC-H SF=10 on S3Summary
Detailed Results Table
|
Contributor
Benchmarks: Statistical and Population GeneticsSummary
Detailed Results Table
|
Contributor
Benchmarks: CompressionSummary
Detailed Results Table
|
Contributor
Benchmarks: Clickbench on NVMESummary
Detailed Results Table
|
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Deploying vortex-bench with
|
| Latest commit: |
6a79c78
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ddb23145.vortex-93b.pages.dev |
| Branch Preview URL: | https://ngates-remove-duplicate-vali.vortex-93b.pages.dev |
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
joseph-isaacs
added
the
action/benchmark
github-actions
bot
removed
the
action/benchmark
Contributor
Benchmarks: FineWeb NVMeSummary
Detailed Results Table
|
Signed-off-by: Nicholas Gates <nick@nickgates.com>
github-actions
bot
removed
the
action/benchmark
Signed-off-by: Nicholas Gates <nick@nickgates.com>
github-actions
bot
removed
the
action/benchmark-sql
| for chunk in array.non_empty_chunks() { | ||
| if !chunk.all_valid() { | ||
| return false; | ||
| match validites.iter().all_equal_value() { |
Signed-off-by: Nicholas Gates <nick@nickgates.com>
|
|
||
| // AND codes_validity with values_valid_mask: | ||
| // position is valid iff the code is valid AND the value it points to is valid | ||
| let validity = and(&codes_validity, &values_valid_mask)?; |
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
github-actions
bot
removed
the
action/benchmark
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Comment on lines
569
to
583
| fn all_valid(&self) -> bool { | ||
| <V::ValidityVTable as ValidityVTable<V>>::all_valid(&self.0) | ||
| match self.validity().vortex_expect("Failed to get validity") { | ||
| Validity::NonNullable | Validity::AllValid => true, | ||
| Validity::AllInvalid => false, | ||
| Validity::Array(a) => a.statistics().compute_min::<bool>().unwrap_or(false), | ||
| } | ||
| } | ||
|
|
||
| fn all_invalid(&self) -> bool { | ||
| <V::ValidityVTable as ValidityVTable<V>>::all_invalid(&self.0) | ||
| match self.validity().vortex_expect("Failed to get validity") { | ||
| Validity::NonNullable | Validity::AllValid => false, | ||
| Validity::AllInvalid => true, | ||
| Validity::Array(a) => !a.statistics().compute_max::<bool>().unwrap_or(true), | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
this looks like it might be very expensive?
| } | ||
| } | ||
|
|
||
| fn valid_count(&self) -> usize { |
Contributor
There was a problem hiding this comment.
This will be null count soon right?
joseph-isaacs
approved these changes
Jan 19, 2026
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
danking
pushed a commit
that referenced
this pull request
Feb 6, 2026
As part of moving towards an explicit execution model, arrays now all return "Validity" which represents the logical validity of the array. scalar_at and min/max operations then work as normal over this validity data structure. Follow ups: - [ ] Remove ValidityVTable, moving `validity` function over to the main Array vtable. --------- Signed-off-by: Nicholas Gates <nick@nickgates.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As part of moving towards an explicit execution model, arrays now all return "Validity" which represents the logical validity of the array. scalar_at and min/max operations then work as normal over this validity data structure.
Follow ups:
validityfunction over to the main Array vtable.