Skip to content

non_nan_check doesn't try to cast arbitrary scalars to primitive#8891

Merged
robert3005 merged 1 commit into
developfrom
rk/statsrewrite
Jul 22, 2026
Merged

non_nan_check doesn't try to cast arbitrary scalars to primitive#8891
robert3005 merged 1 commit into
developfrom
rk/statsrewrite

Conversation

@robert3005

Copy link
Copy Markdown
Contributor

non_nan_check tried to unconditionally cast Scalar to PrimitiveScalar - these
methods are error producing but in this case we never care about error and want
to ignore those cases. It's cheaper to directly check dtype for float type

Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005 robert3005 added the changelog/fix A bug fix label Jul 21, 2026
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Polar Signals Profiling Results

Latest Run

Status Commit Job Attempt Link
🟢 Done 0086542 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: +0.4%
Engines: DataFusion No clear signal (-1.7%, low confidence) · DuckDB No clear signal (+2.5%, low confidence)
Vortex (geomean): 0.972x ➖
Parquet (geomean): 0.970x ➖
Shifts: Parquet (control) -3.0% · Median polish -3.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.930x ➖, 0↑ 0↓)
name PR 0086542 (ns) base 5e8917c (ns) ratio (PR/base)
vortex_q00/datafusion:vortex-file-compressed 9932950 10381533 0.96
vortex_q01/datafusion:vortex-file-compressed 6200217 6857576 0.90
datafusion / parquet (0.947x ➖, 0↑ 0↓)
name PR 0086542 (ns) base 5e8917c (ns) ratio (PR/base)
vortex_q00/datafusion:parquet 20584810 21125211 0.97
vortex_q01/datafusion:parquet 4810870 5231260 0.92
duckdb / vortex-file-compressed (1.019x ➖, 0↑ 0↓)
name PR 0086542 (ns) base 5e8917c (ns) ratio (PR/base)
vortex_q00/duckdb:vortex-file-compressed 10114747 10147371 1.00
vortex_q01/duckdb:vortex-file-compressed 6148375 5904015 1.04
duckdb / parquet (0.994x ➖, 0↑ 0↓)
name PR 0086542 (ns) base 5e8917c (ns) ratio (PR/base)
vortex_q00/duckdb:parquet 23496777 23924449 0.98
vortex_q01/duckdb:parquet 9383100 9335080 1.01

No file size changes detected.

@robert3005
robert3005 enabled auto-merge (squash) July 22, 2026 01:25
@robert3005
robert3005 merged commit 0c1f184 into develop Jul 22, 2026
111 of 113 checks passed
@robert3005
robert3005 deleted the rk/statsrewrite branch July 22, 2026 01:28
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.

2 participants