Skip to content

Fix nested scalar hashing#8804

Open
connortsui20 wants to merge 1 commit into
developfrom
agent/fix-scalar-nested-hashing
Open

Fix nested scalar hashing#8804
connortsui20 wants to merge 1 commit into
developfrom
agent/fix-scalar-nested-hashing

Conversation

@connortsui20

@connortsui20 connortsui20 commented Jul 16, 2026

Copy link
Copy Markdown
Member

Fixes #8744

Scalar equality ignores dtype nullability recursively, but hashing only removed top-level nullability. The typed scalar views had the same mismatch, so equal nested values could produce different hashes.

  • adds recursive nullability-insensitive dtype hashing matching DType::eq_ignore_nullability
  • uses it for Scalar, ListScalar, StructScalar, and ExtScalar
  • covers nested list, struct, Union dtype, and extension storage nullability regressions

This is kind of ugly but I don't think there is a better way to do this

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.65217% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.82%. Comparing base (17f843a) to head (76745ac).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
vortex-array/src/dtype/dtype_impl.rs 87.50% 6 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.

@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 76745ac 1 Explore Profiling Data

Powered by Polar Signals Cloud

@github-actions

Copy link
Copy Markdown
Contributor

Benchmarks: Vortex queries

Verdict: No clear signal (low confidence)
Attributed Vortex impact: +5.6%
Engines: DataFusion Likely regression (+12.6%, medium confidence) · DuckDB No clear signal (-0.9%, low confidence)
Vortex (geomean): 1.008x ➖
Parquet (geomean): 0.981x ➖
Shifts: Parquet (control) -1.9% · Median polish +1.1%

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 (1.067x ➖, 0↑ 0↓)
name PR 76745ac (ns) base 17f843a (ns) ratio (PR/base)
vortex_q00/datafusion:vortex-file-compressed 10302567 9877069 1.04
vortex_q01/datafusion:vortex-file-compressed 6927132 6345790 1.09
datafusion / parquet (0.948x ➖, 0↑ 0↓)
name PR 76745ac (ns) base 17f843a (ns) ratio (PR/base)
vortex_q00/datafusion:parquet 20891906 21920309 0.95
vortex_q01/datafusion:parquet 4840236 5135545 0.94
duckdb / vortex-file-compressed (1.007x ➖, 0↑ 0↓)
name PR 76745ac (ns) base 17f843a (ns) ratio (PR/base)
vortex_q00/duckdb:vortex-file-compressed 10738963 10602435 1.01
vortex_q01/duckdb:vortex-file-compressed 6564743 6559583 1.00
duckdb / parquet (1.016x ➖, 0↑ 0↓)
name PR 76745ac (ns) base 17f843a (ns) ratio (PR/base)
vortex_q00/duckdb:parquet 23928956 23596214 1.01
vortex_q01/duckdb:parquet 9711741 9543018 1.02

No file size changes detected.

@connortsui20
connortsui20 marked this pull request as ready for review July 16, 2026 20:31
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.

Scalar hashing violates Eq for nested nullability

1 participant