Skip to content

Bypass unknown aggregates in ZonedLayout#8904

Open
connortsui20 wants to merge 1 commit into
developfrom
ct/ignore-unknown-aggregate
Open

Bypass unknown aggregates in ZonedLayout#8904
connortsui20 wants to merge 1 commit into
developfrom
ct/ignore-unknown-aggregate

Conversation

@connortsui20

Copy link
Copy Markdown
Member

Rationale for this change

Tracking issue: #8901

Resolves the unknown aggregate subissue.

What changes are included in this PR?

Allows us to ignore unknown aggregates in ZonedLayout by simply bypassing any pruning. Even if the query might be slow because of this (it will be a full scan), it is better than erroring.

What APIs are changed? Are there any user-facing changes?

Changes to_aggregate_fn to to_aggregate_fn_opt and aggregate_fns_from_specs to try_aggregate_fns_from_specs

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20 connortsui20 added the changelog/fix A bug fix label Jul 22, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Polar Signals Profiling Results

Latest Run

Status Commit Job Attempt Link
🟢 Done 9c2829e 1 Explore Profiling Data

Powered by Polar Signals Cloud

@github-actions

Copy link
Copy Markdown
Contributor

Benchmarks: Vortex queries 📖

Verdict: Likely regression (high confidence)
Attributed Vortex impact: +30.8%
Engines: DataFusion Likely regression (+36.9%, environment too noisy confidence) · DuckDB Likely regression (+24.9%, high confidence)
Vortex (geomean): 1.338x ❌
Parquet (geomean): 1.170x ❌
Shifts: Parquet (control) +17.0% · Median polish +30.8%

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.698x ❌, 0↑ 2↓)
name PR 9c2829e (ns) base 57962f4 (ns) ratio (PR/base)
vortex_q00/datafusion:vortex-file-compressed 🚨 13356591 8197567 1.63
vortex_q01/datafusion:vortex-file-compressed 🚨 8647756 4885078 1.77
datafusion / parquet (1.241x ❌, 0↑ 2↓)
name PR 9c2829e (ns) base 57962f4 (ns) ratio (PR/base)
vortex_q00/datafusion:parquet 🚨 23999705 20638806 1.16
vortex_q01/datafusion:parquet 🚨 6062155 4580085 1.32
duckdb / vortex-file-compressed (1.379x ❌, 0↑ 2↓)
name PR 9c2829e (ns) base 57962f4 (ns) ratio (PR/base)
vortex_q00/duckdb:vortex-file-compressed 🚨 14142553 10061599 1.41
vortex_q01/duckdb:vortex-file-compressed 🚨 8346787 6167184 1.35
duckdb / parquet (1.104x ❌, 0↑ 1↓)
name PR 9c2829e (ns) base 57962f4 (ns) ratio (PR/base)
vortex_q00/duckdb:parquet 🚨 25966664 23195009 1.12
vortex_q01/duckdb:parquet 10171388 9340266 1.09

No file size changes detected.

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.

1 participant