Skip to content

refactor(sql): separate aggregate registration and reuse in binder#19579

Merged
forsaken628 merged 20 commits intodatabendlabs:mainfrom
forsaken628:aggr-bind
Mar 27, 2026
Merged

refactor(sql): separate aggregate registration and reuse in binder#19579
forsaken628 merged 20 commits intodatabendlabs:mainfrom
forsaken628:aggr-bind

Conversation

@forsaken628
Copy link
Copy Markdown
Collaborator

@forsaken628 forsaken628 commented Mar 19, 2026

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

  • Rework aggregate binding around a unified AggregateInfo registry so builtin aggregates, UDAFs, group items, and grouping metadata follow the same binder path.
  • Split aggregate rewriting into two explicit modes: one that can register new aggregate/UDAF calls during aggregate analysis, and one that can only reuse already-bound aggregate results in later phases.
  • Tighten clause semantics across WINDOW, QUALIFY, ORDER BY, and ProjectSet by preventing late aggregate creation, fixing alias visibility, and making aggregate arguments resolve against the right scope.
  • Stabilize mixed aggregate/SRF and grouped/window cases, including lazy vs early ProjectSet, grouped alias reuse, named window inheritance checks, and GROUPING SETS / CUBE / ROLLUP / grouping() rewriting.
  • Add a query-local UDF cache in BindContext and expand golden/integration coverage for binder, window, identifier, and optimizer regressions.

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@github-actions github-actions Bot added the pr-refactor this PR changes the code base without new features or bugfix label Mar 19, 2026
@forsaken628 forsaken628 changed the title refactor(sql): Aggr bind refactor(sql): separate aggregate registration and reuse in binder Mar 24, 2026
@forsaken628 forsaken628 marked this pull request as ready for review March 24, 2026 05:36
@forsaken628 forsaken628 requested review from b41sh and sundy-li March 24, 2026 05:37
Copy link
Copy Markdown

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

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: f4e00ba538

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/query/sql/src/planner/binder/window.rs
Comment thread src/query/sql/src/planner/semantic/grouping_check.rs Outdated
Comment thread src/query/sql/src/planner/semantic/grouping_check.rs Outdated
@forsaken628 forsaken628 requested a review from b41sh March 27, 2026 01:42
@forsaken628 forsaken628 merged commit 51eea95 into databendlabs:main Mar 27, 2026
90 checks passed
@forsaken628 forsaken628 deleted the aggr-bind branch March 27, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-refactor this PR changes the code base without new features or bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants