Skip to content

Canonical variant array#7026

Merged
AdamGS merged 5 commits intodevelopfrom
adamg/new-variant-canonical
Mar 19, 2026
Merged

Canonical variant array#7026
AdamGS merged 5 commits intodevelopfrom
adamg/new-variant-canonical

Conversation

@AdamGS
Copy link
Contributor

@AdamGS AdamGS commented Mar 18, 2026

This PR introduces a minimal new canonical array for the new variant DType, its not very useable at the moment but I figured its a piece of mostly boilerplate code that is worth splitting out of the bigger branch.

Signed-off-by: Adam Gutglick <adam@spiraldb.com>
@AdamGS AdamGS changed the title Canonical variant array [WIP] Canonical variant array Mar 18, 2026
@AdamGS AdamGS added the changelog/feature A new feature label Mar 18, 2026
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 18, 2026

Merging this PR will degrade performance by 12.34%

❌ 1 regressed benchmark
✅ 1008 untouched benchmarks
⏩ 1515 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation bench_many_nulls[0.9] 463.1 µs 528.4 µs -12.34%

Comparing adamg/new-variant-canonical (03006db) with develop (20070d2)

Open in CodSpeed

Footnotes

  1. 1515 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.

AdamGS added 3 commits March 18, 2026 19:56
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
@AdamGS AdamGS requested review from connortsui20 and gatesn March 19, 2026 13:06
@AdamGS AdamGS marked this pull request as ready for review March 19, 2026 13:06
@AdamGS AdamGS changed the title [WIP] Canonical variant array Canonical variant array Mar 19, 2026
}

/// Creates a new VariantArray with the given nullability.
pub fn new_nullable(child: ArrayRef, nullability: Nullability) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know that VariantArray is going to have a child ArrayRef or is that just so you can implement some of the required methods?

Also, we probably just want new to take a Nullablity directly instead of having a new_nullable to be consistent with the other nested arrays.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The general idea is that the variant array has a single child which should be a more concrete encoding of the "variant" family.

Copy link
Contributor Author

@AdamGS AdamGS Mar 19, 2026

Choose a reason for hiding this comment

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

also fixed the new thing

Signed-off-by: Adam Gutglick <adam@spiraldb.com>

fn stats(array: &Self::Array) -> StatsSetRef<'_> {
array.stats_set.to_ref(array.as_ref())
array.child.statistics()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is the right thing here, might change as I go deeper into the specific encoding

@AdamGS AdamGS merged commit 683ba3a into develop Mar 19, 2026
101 of 103 checks passed
@AdamGS AdamGS deleted the adamg/new-variant-canonical branch March 19, 2026 13:38
dimitarvdimitrov pushed a commit that referenced this pull request Mar 20, 2026
This PR introduces a minimal new canonical array for the new variant
DType, its not very useable at the moment but I figured its a piece of
mostly boilerplate code that is worth splitting out of the bigger
branch.

---------

Signed-off-by: Adam Gutglick <adam@spiraldb.com>
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