Skip to content

Add UnionValue and UnionScalar#8791

Closed
connortsui20 wants to merge 1 commit into
developfrom
agent/union-scalar-nullability
Closed

Add UnionValue and UnionScalar#8791
connortsui20 wants to merge 1 commit into
developfrom
agent/union-scalar-nullability

Conversation

@connortsui20

@connortsui20 connortsui20 commented Jul 16, 2026

Copy link
Copy Markdown
Member

Tracking issue: #8769

Adds scalar implementation for Union type in Vortex.

Changes

Adds UnionValue which exists inside of ScalarValue and the typed view UnionScalar<'a>.

pub struct UnionValue {
    /// The type ID selecting a variant in the enclosing [`DType::Union`].
    type_id: u8,

    /// The selected variant scalar.
    ///
    /// This is boxed to break the recursive layout between [`ScalarValue`]
    /// and [`UnionValue`].
    value: `Option<Box<ScalarValue>>`,
}

I was debating if we should instead store a Option<Box<ScalarValue>> instead of just Box<Scalar> (which carries an extra dtype and an extra allocation even if the value is null). I think it is nice to have the dtype in there for implementation ease. Not sure if it's worth it.

We use Option<Box<ScalarValue>> instead of Box<Scalar> so that we do not store DType in multiple places. If we did so, we run into synchronization issues with casting.

For casting, this only allows identity and null casting (like everything else), otherwise it just panics with unimplemented. We probably want to figure this out later as it is quite valuable to cast union types.

Finally, the "default value" and "zero value" semantics here are as such:

  • default_value(nullable Union) -> returns an outer null.
  • default_value(non-nullable Union) -> panics because there is no designated default variant.
  • zero_value(Union) -> panics because a Union has no canonical zero variant.
  • is_zero() on a null Union -> returns None.
  • is_zero() on a valid Union -> returns Some(false), even when its child is numerically zero.

Note that none of these behaviors depend on variant ordering. And also I do think that it is possible that we just choose the first variant of a union to be the default, I think this is kind of opinionated, and it also doesn't solve the issue where there are 0 variants (empty union / void type). So I would prefer if we do this in a separate change.

@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch from 547f89b to a05b340 Compare July 16, 2026 14:59
@connortsui20
connortsui20 force-pushed the agent/union-type-ids-u8 branch from 7820b66 to d9583d3 Compare July 16, 2026 15:09
@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch from a05b340 to bb2292d Compare July 16, 2026 15:10
@connortsui20
connortsui20 force-pushed the agent/union-type-ids-u8 branch from d9583d3 to 2c72940 Compare July 16, 2026 15:46
@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch from bb2292d to 6c0a2ea Compare July 16, 2026 15:47
@connortsui20
connortsui20 force-pushed the agent/union-type-ids-u8 branch from 2c72940 to 378b981 Compare July 16, 2026 16:02
@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch 2 times, most recently from 056c755 to 877e544 Compare July 16, 2026 16:08
@connortsui20
connortsui20 force-pushed the agent/union-type-ids-u8 branch from 378b981 to 2eb8864 Compare July 16, 2026 16:08
Base automatically changed from agent/union-type-ids-u8 to develop July 16, 2026 16:48
@connortsui20 connortsui20 changed the title Add outer nullability to Union scalars Add UnionValue and UnionScalar Jul 16, 2026
@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch from 877e544 to af088ad Compare July 16, 2026 17:23
@codspeed-hq

codspeed-hq Bot commented Jul 16, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 11.1%

⚡ 1 improved benchmark
❌ 2 regressed benchmarks
✅ 1667 untouched benchmarks
⏩ 53 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation copy_nullable[65536] 1 ms 1.4 ms -24.25%
Simulation copy_non_nullable[65536] 908.8 µs 1,089.3 µs -16.57%
Simulation true_count_vortex_buffer[128] 580.6 ns 522.2 ns +11.17%

Tip

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


Comparing agent/union-scalar-nullability (dd92c91) with develop (e6eeaae)

Open in CodSpeed

Footnotes

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

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Polar Signals Profiling Results

Latest Run

Status Commit Job Attempt Link
🟢 Done dd92c91 1 Explore Profiling Data
Previous Runs (9)
Status Commit Job Attempt Link
🟢 Done c812c67 1 Explore Profiling Data
🟢 Done df88950 1 Explore Profiling Data
🟢 Done 42124e3 1 Explore Profiling Data
🟢 Done e992375 1 Explore Profiling Data
🟢 Done f410d02 1 Explore Profiling Data
🟢 Done abefbe3 1 Explore Profiling Data
🟢 Done d5458d2 1 Explore Profiling Data
🟢 Done 3a22a55 1 Explore Profiling Data
🟢 Done af088ad 1 Explore Profiling Data

Powered by Polar Signals Cloud

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Benchmarks: Vortex queries 📖

Verdict: No clear signal (low confidence)
Attributed Vortex impact: +2.6%
Engines: DataFusion No clear signal (+1.6%, low confidence) · DuckDB No clear signal (+3.5%, low confidence)
Vortex (geomean): 0.904x ➖
Parquet (geomean): 0.893x ✅
Shifts: Parquet (control) -10.7% · Median polish -10.0%

How to read Verdict and Engines
  • Verdict: Overall PR-level signal after subtracting baseline drift estimated from Parquet control rows. It can be Likely improvement, Likely regression, or No clear signal.
  • Engines: Per-engine attribution. DataFusion is compared against DataFusion/Parquet controls; DuckDB is compared against DuckDB/Parquet controls. This answers whether each engine improved or regressed independently.
  • Confidence: Based on directional consistency, share of rows above the noise floor, and control-run noise.

datafusion / vortex-file-compressed (0.846x ✅, 2↑ 0↓)
name PR dd92c91 (ns) base e6eeaae (ns) ratio (PR/base)
vortex_q00/datafusion:vortex-file-compressed 🚀 9894371 11446321 0.86
vortex_q01/datafusion:vortex-file-compressed 🚀 6171790 7449028 0.83
datafusion / parquet (0.833x ✅, 2↑ 0↓)
name PR dd92c91 (ns) base e6eeaae (ns) ratio (PR/base)
vortex_q00/datafusion:parquet 🚀 20122031 23584739 0.85
vortex_q01/datafusion:parquet 🚀 4564816 5616200 0.81
duckdb / vortex-file-compressed (0.991x ➖, 0↑ 0↓)
name PR dd92c91 (ns) base e6eeaae (ns) ratio (PR/base)
vortex_q00/duckdb:vortex-file-compressed 9900379 10387695 0.95
vortex_q01/duckdb:vortex-file-compressed 6066323 5885688 1.03
duckdb / parquet (0.957x ➖, 0↑ 0↓)
name PR dd92c91 (ns) base e6eeaae (ns) ratio (PR/base)
vortex_q00/duckdb:parquet 23310943 24576114 0.95
vortex_q01/duckdb:parquet 9397600 9723548 0.97

No file size changes detected.

@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch from af088ad to 3a22a55 Compare July 16, 2026 17:36
@connortsui20
connortsui20 changed the base branch from develop to agent/union-outer-nullability July 16, 2026 17:38
@connortsui20 connortsui20 added the changelog/feature A new feature label Jul 16, 2026
@connortsui20
connortsui20 force-pushed the agent/union-outer-nullability branch from c5f26f5 to 7db4fac Compare July 16, 2026 17:43
@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch from 3a22a55 to 50d65b5 Compare July 16, 2026 17:43
@connortsui20
connortsui20 force-pushed the agent/union-outer-nullability branch from 7db4fac to f04a9d8 Compare July 16, 2026 18:54
@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch 3 times, most recently from 5b5d8c0 to 39be7e2 Compare July 16, 2026 20:01
Base automatically changed from agent/union-outer-nullability to develop July 16, 2026 20:20
@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch 2 times, most recently from d5458d2 to abefbe3 Compare July 17, 2026 15:39
@connortsui20
connortsui20 marked this pull request as ready for review July 17, 2026 18:15

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: abefbe3917

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vortex-array/src/scalar/scalar_impl.rs Outdated
@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch 3 times, most recently from e992375 to 42124e3 Compare July 17, 2026 19:37
@connortsui20
connortsui20 marked this pull request as draft July 17, 2026 20:52
@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch 2 times, most recently from df88950 to c812c67 Compare July 17, 2026 21:31
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20
connortsui20 force-pushed the agent/union-scalar-nullability branch from c812c67 to dd92c91 Compare July 17, 2026 21:44
@connortsui20
connortsui20 deleted the agent/union-scalar-nullability branch July 17, 2026 21:54
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.87805% with 83 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.03%. Comparing base (e6eeaae) to head (dd92c91).

Files with missing lines Patch % Lines
vortex-array/src/scalar/tests/casting.rs 89.38% 19 Missing ⚠️
vortex-array/src/scalar/tests/primitives.rs 91.02% 14 Missing ⚠️
vortex-array/src/scalar/typed_view/union.rs 92.00% 14 Missing ⚠️
vortex-array/src/scalar/proto.rs 90.35% 11 Missing ⚠️
vortex-array/src/scalar/scalar_value.rs 70.58% 10 Missing ⚠️
vortex-array/src/scalar/downcast.rs 65.00% 7 Missing ⚠️
vortex-array/src/scalar/display.rs 87.50% 3 Missing ⚠️
...tex-array/src/arrays/constant/vtable/operations.rs 90.47% 2 Missing ⚠️
vortex-array/src/scalar/validate.rs 95.55% 2 Missing ⚠️
vortex-array/src/scalar/scalar_impl.rs 95.65% 1 Missing ⚠️

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

1 participant