Skip to content

SQL: Add rule for merging nested Aggregates.#18498

Merged
gianm merged 2 commits intoapache:masterfrom
gianm:sql-aggregate-merge-rule
Jan 29, 2026
Merged

SQL: Add rule for merging nested Aggregates.#18498
gianm merged 2 commits intoapache:masterfrom
gianm:sql-aggregate-merge-rule

Conversation

@gianm
Copy link
Contributor

@gianm gianm commented Sep 8, 2025 •
edited
Loading

The rule is adapted from Calcite's AggregateMergeRule, with two changes:

  1. Includes a workaround for https://issues.apache.org/jira/browse/CALCITE-7162

  2. Includes the ability to merge two Aggregate with a Project between them, by pushing the Project below the new merged Aggregate.

The goal is to rewrite a query like:

SELECT
  hr,
  UPPER(t1.x) x,
  SUM(t1.cnt) cnt,
  MIN(t1.mn) mn,
  MAX(t1.mx) mx
FROM (
  SELECT
    floor(__time to hour) hr,
    dim2 x,
    COUNT(*) cnt,
    MIN(m1 * 5) mn,
    MAX(m1 + m2) mx
  FROM druid.foo
  WHERE dim2 IN ('abc', 'def', 'a', 'b', '')
  GROUP BY 1, 2
) t1
WHERE t1.x IN ('abc', 'foo', 'bar', 'a', '')
GROUP BY 1, 2

Into:

SELECT
  FLOOR(__time TO hour) hr,
  UPPER(dim2) x,
  COUNT(*) cnt,
  MIN(m1 * 5) mn,
  MAX(m1 + m2) mx
FROM druid.foo
WHERE dim2 IN ('abc', 'a', '')
GROUP BY 1, 2

@gianm
SQL: Add rule for merging nested Aggregates.
0f71ea8 
The rule is adapted from Calcite's AggregateMergeRule, with two changes:

1) Includes a workaround for https://issues.apache.org/jira/browse/CALCITE-7162

2) Includes the ability to merge two Aggregate with a Project between
   them, by pushing the Project below the new merged Aggregate.
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 8, 2025
View reviewed changes
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 9, 2025
View reviewed changes
@cryptoe cryptoe requested a review from kgyrtkirk September 9, 2025 10:13
@github-actions
Copy link

github-actions bot commented Nov 9, 2025

This pull request has been marked as stale due to 60 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If you think
that's incorrect or this pull request should instead be reviewed, please simply
write any comment. Even if closed, you can still revive the PR at any time or
discuss it on the dev@druid.apache.org list.
Thank you for your contributions.

@github-actions github-actions bot added the stale label Nov 9, 2025
@github-actions
Copy link

github-actions bot commented Dec 7, 2025

This pull request/issue has been closed due to lack of activity. If you think that
is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions bot closed this Dec 7, 2025
@gianm gianm reopened this Dec 31, 2025
@github-actions github-actions bot removed the stale label Jan 1, 2026
clintropolis
clintropolis approved these changes Jan 29, 2026
View reviewed changes
Copy link
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

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

👍 these plans look nicer to me

@gianm gianm merged commit 0e2f86f into apache:master Jan 29, 2026
110 checks passed
@gianm gianm deleted the sql-aggregate-merge-rule branch January 29, 2026 22:34
kgyrtkirk added a commit to kgyrtkirk/druid that referenced this pull request Feb 9, 2026
@kgyrtkirk @capistrant
@gianm @GWphua @clintropolis @Fly-Style @vogievetsky @puzpuzpuz @jtuglu1 @cecemei @dependabot @317brian @mshahid6 @kfaraz @aho135 @abhishekrb19 @ashwintumma23
apache/master->master: 9e2fa21 (#1198)
f9e6528 
* Implement a fingerprinting mechanism to track compaction states in a more efficient manner (apache#18844)

* meatadata store bits part 1

* annotate segments with compaction fingerprint before persist

* Add ability to generate compaction state fingerprint

* add fingerprint to task context and make legacy last compaction state storage configurable

* update embedded tests for compaction supervisors to flex fingerprints

* checkpoint with persisting compaction states

* add duty to clean up unused compaction states

* take fingerprints into account in CompactionStatus

* Add and improve tests

* get rid of some todo comments

* fix checkstyle

* cleanup some more TODO

* Add some docs

* update web console

* make cache size configurable and fix some spelling

* fixup use of deprecated builder

* fix checktyle

* fix coordinator compactsegments duty and respond to self review comments

* fix spellchecker

* predates is a word

* improve some javadocs

* simplify some test assertions based on review

* better naming

* controller impl cleanup

* For compaction supervisors, take persisting pending compaction states out of hot path

* use Configs.valueOrDefault helper in data segment

* Refactor where fingerprinting happens and how the object mapper is wired up

* refactor CompactionStateManager into an interface with a persisted and heap impl

* remove fingerprinting support from the coordinator compact segments duty

* Move on heap compaction state manager to test sources

* CompactionStateManager is now overlord only

* Refactor how the compaction state fingerprint cache is wired up

* prettify

* small changes after self-review

* Cleanup CompactionStateCache per review

* compactionstatemanager to compactionstatestorage plus refactor

* Add compaction state added and deleted metrics

* improve queries for compaction state cache sync

* clean up doc wording

* Miscl. cleanup from review

* some metadata store code cleanup

* refactor id out of the compaction states table as it is superflous

* Some CompactionStatus cleanup

* Migrate the location of creating a compaction state from config

* More refactoring per review

* refactor to remove duplicate fingerprint generator code

* Do some consolidation of fingerprint related classes to clean up code

* minor cleanup

* fix fobidden api use

* Improvements and cleanup to the fingerprint and state persist + cache

* Refactor where in the code compaction fingerprints are generated

* Formalize unique constraint exception check in sqlmetadataconnector and db specific impls

* some naming cleanup

* Migrate the compaction state cleanup duty to the overlord

* Blow up the compaction supervisor scheduler if incremental caching is disabled

* add some strict input sanitization in upserting compaction fingerprints

* cleanup test class

* Add pending flag to compaction state to prevent potentially destructive early cleanup

* Refactor database naming to use indexingState instead of compactionState

* Refactor naming to IndexingState for the metadata cleanup duty

* refresh some docs

* fixup tests

* Refactoring name of CompactionStateCache to IndexingStateCache

* Rename CompactionStateStorage to IndexingStateStorage

* Refactor compactionStateFingerprint out of the code in favor of indexingStateFingerprint

* Refactor FingerprintMapper name to remove compaction for indexing state

* refactorings after self review

* fixup a few things post merge with master

* Cleanup and refactor after code review round

Batch marking of indexing states as active to avoid chained updates where only one is needed

Build segments table missing columns error column by column

refactor how we are configuring and executing the ol metadata cleanup duties.

fix missed naming refactor

Improve readability of upsertIndexingState

Fixup SqlIndexingStateStorage constructor

drop default impl of isUniqueConstraintViolation

Refactor how the deterministic mapper is handled for reindexing

* cleanup

* use effective state for dimspec and indexspec for reindexing fingerprinting

* Only call into running checks if there are unknown states to check

* Update milestone on PR close and ensure they are visible for the originally desired milestone (apache#18935)

* SegmentLocalCacheManagerConcurrencyTest: Use tempDir for temp files. (apache#18937)

The tests should use temporary directories rather than the current
working directory.

* Update to testcontainers 2.x and update various images. (apache#18945)

This patch updates to testcontainers 2.x, which improves compatibility
with newer versions of Docker. It also updates most images to the latest
versions available. PostgreSQL and MariaDB remain on 16 and 11, however.

* Max metrics for group by queries (apache#18934)

Added metrics mergeBuffer/maxAcquisitionTimeNs, groupBy/maxSpilledBytes and groupBy/maxMergeDictionarySize to track peak resource usage per query.

* fix json column isNumeric check to properly consider array element selector types (apache#18948)

* Add sys.queries table. (apache#18923)

The sys.queries table provides insight into currently-running queries.
It provides the same information as the /druid/v2/sql/queries API. As such,
it currently only works with Dart.

In this patch the table is documented, but off by default. It can be
enabled by setting druid.sql.planner.enableSysQueriesTable = true.

This patch additionally adds an "includeComplete" parameter to
/druid/v2/sql/queries, which is used by the implementation of the
sys.queries table, to allow it to show information for recently-completed
queries.

* Bump kubernetes-client to latest and level vertx with what kubernetes-client uses (apache#18947)

* Adjust cost-based autoscaler algorithm (apache#18936)

* use includeComplete (apache#18940)

* Add configurable option to scale-down during task run time for cost-based autoscaler (apache#18958)

* Add configurable option to scale-down during task run time for cost-based autoscaler

* Docs

* Address review comments, compress tests a bit

* remove custom json serde for DataNodeService (apache#18961)

* Faster bucket search in ByteBufferHashTable (apache#18952)

Adds hash code comparison for large enough keys to ByteBufferHashTable#findBucket(). Also, changes key comparison to use long/int/byte instead of byte-only comparison (thus, the comparison is now closer to HashTableUtils#memoryEquals() used in MemoryOpenHashTable). These changes are aimed to speed-up bucket search in ByteBufferHashTable, especially in high-collision cases.

* Allow failing on residual for Iceberg filters on non-partition cols (apache#18953)

Currently Iceberg ingest extension may ingest more data than is necessary due to residual data occurring from an Iceberg filter on non-partition columns. This adds an option to ignore + log a warning or fail on filters that result in residual so users are aware of this extra data and can action on it.

* Rely on `taskCountMin` in `computeValidTaskCounts`; correct the embedded test for cost-based-autoscaler (apache#18963)

This patch fixes a behaviour where computeValidTaskCounts took care of upper bound (taskCountMax), but did not care about taskCountMin.

Also it fixes a flaky embedded test.

* Web console: Server props dialog (apache#18960)

* Init server props table

* Add trim starts

* reformat

* Update `TableInputSpec` to be able to handle specific segments. (apache#18922)

* input

* format and deprecate

* allow non-complete segments

* test

* SQL: Add rule for merging nested Aggregates. (apache#18498)

The rule is adapted from Calcite's AggregateMergeRule, with two changes:

1) Includes a workaround for https://issues.apache.org/jira/browse/CALCITE-7162

2) Includes the ability to merge two Aggregate with a Project between
   them, by pushing the Project below the new merged Aggregate.

* Speed up TopNQueryRunnerTest. (apache#18955)

Takes the runtime from ~3 minutes to 10 seconds by reducing the number
of test runs by 32x. There are two changes:

1) Instead of parameterizing for every possible combination of
   monomorphic specialization flags, only parameterize for all-on and
   all-off. The specializations handle different cases anyway, so they
   wouldn't trigger on the same queries. Reduces number of test runs by
   16x.

2) Remove the parameterization on duplicateSingleAggregatorQueries. Only
   a handful of tests used it. Instead of parameterizing the entire
   suite, that handful of tests is expanded to include
   _duplicateAggregators versions. Reduces number of test runs by 2x.

* Fix Hadoop multi-value string null value handling to match native batch (apache#18944)

Doing some more digging, I found another unfortunate data difference between native batch (on-cluster) and Hadoop batch ingest. Ingesting a multi-value string ["a","b",null] with Hadoop is treated as ["a","b","null"] and in native batch, this correctly ingests to ["a","b",null]. This is difference appears to be a bug in all Druid versions(even latest). While this will not affect the current null handling migration, this will affect the future Hadoop -> native batch ingestion migration that will also need to take place.

Hadoop doesn't allow for all-null columns in segments, it simply excludes them from the segment. I've updated the Hadoop job to support running druid.indexer.task.storeEmptyColumns=true, which allows us to store all NULL columns (how native/streaming ingest work today).

BREAKING CHANGES
1. Hadoop ingests will now process multi-value string inputs like ["a","b",null] -> ["a","b",null] instead of ["a","b","null"] to match native batch ingestion.

2. Hadoop ingests will now by default keep columns with all NULL values, instead of excluding them from the segment.
useStringValueOfNullInLists parameter in RowBasedColumnSelectorFactory.java‎ has been removed.

* modify ExprEvalBindingVector to use current vector size instead of array length when coercing values, cache coercion arrays (apache#18967)

* modify ExprEvalBindingVector to use current vector size instead of array length when coercing values, cache coercsion arrays

expression vector binding improvements

changes:
* split ExpressionEvalBindingVector into ExpressionEvalNumericBindingVector and ExpressionEvalObjectBindingVector
* modify ExpressionEvalNumericBindingVector and ExpressionEvalObjectBindingVector to use current vector size instead of input array size when coercing values
* modify ExpressionEvalNumericBindingVector and ExpressionEvalObjectBindingVector to use externally managed object array caches for value coercion instead of recreating each time

* benchmarks

* SQL: Use specialized virtual columns for expression filters. (apache#18965)

This patch adjusts planning for expression filters to use specialized
virtual columns when they exist. This allows them to take advantage
of optimizations, such as the ones that are available for JSON_VALUE,
even when the overall expression is complicated.

* add tier/storage/capacity metric to make actual tier disk size metrics available for historicals in vsf mode (apache#18962)

* Adjust costs for burst scaleup during heavy lag for cost-based autoscaler (apache#18969)

* udpate copyright year to 2026 (apache#18972)

* Bump diff from 4.0.1 to 4.0.4 in /web-console (apache#18933)

* docs: add docs for projections (apache#18056)

* Better query error classification for user errors (apache#18949)

This change checks instanceof before casting RexLiteral.value() to Number in SQL aggregators. When users pass invalid queries (e.g., a string literal '99.99' where numeric literals are expected), InvalidSqlInput exception is thrown, which returns 400 (USER/INVALID_INPUT) instead of 500 (ADMIN/UNCATEGORIZED). This improves error diagnostics for invalid queries.

* changes related to 36 release (apache#18975)

* add vsf AcquireSegmentResult metrics to ChannelCounters (apache#18971)

* Migrate query integration tests to embedded framework (apache#18978)

Changes
---------
- Move `ITBroadcastJoinQueryTest` to embedded framework
- Remove `ITWikipediaQueryTest`
- Add `QueryLaningTest` which was the only useful assertion being done in the wikipedia test

* Upgrade compiler version to JDK 17 (apache#18977)

Upgrade compiler version to JDK 17. This removes compiler compatibility for indexing-hadoop (no longer supported extension).

* add storage_size to sys.servers (apache#18979)

* bugfix: Fix bug that could lead to illegal k8s label ending in non-alphanumeric (apache#18981)

* Remove experimental flag from multi-supervisor docs (apache#18983)

Multi-supervisor support has been in 2 major versions (with v36 being the 3rd). I think the implementation is stable enough for marking as non-experimental.

* Add groupby max metrics to prometheus config (apache#18970)

* Add metrics and improve logging for row signature flapping. (apache#18966)

Add segment/schemaCache/rowSignature/changed and segment/schemaCache/rowSignature/column/count metrics to get visibility into when the Broker's segment metadata cache's row signature for each datasource is initialized and updated.

The rationale for these metrics and logging enhancements is that we noticed row signatures flapping (columns reordered spuriously) that can cause SQL queries to be translated to incorrect native queries because the signatures flapped. This can cause sporadic missing data when the queries are incorrectly planned and is noticeable in environments with high QPS.

* bugfix: Create tombstones when needed while doing REPLACE mode with range partitioning plus parallel indexing (apache#18938)

* Create tombstones for range and hashed partitioning when everything has been filtered out

* MSQ compaction doesn't support hash partitioning

* cleanup test file

* Cleanup verbose comments in test code

* Hashed partitioning doesn't actually need the special handling

* fix checkstyle

* test coverage

* fix vsf load time to be actual load time and not include wait time (apache#18988)

* Update guice to 6.0.0 (apache#18986)

* Update surefire to 3.5.4 ; upgrade NestedDataScanQueryTest to use junit5 (apache#18847)

* Add optional plugins to basic cost function in CostBasedAutoScaler (apache#18976)

Changes:
- separate the logic of pure cost function, making all additional logic opt-in in config;
- `scaleDownBarrier` has been changed to `minScaleDownDelay`, which is now `Duration`;
- changes to high lag fast scaleup: logarithmic scaling formula for idle decay on high lag and task boundaries.

Details:
This change replaces the sqrt-based scaling formula with a logarithmic formula that provides
more aggressive emergency recovery at low task counts and millions of lag.

Idle decay: ` ln(lagSeverity) / ln(maxSeverity)`. Less aggressive, scales well with lag growth.

Formula `K = P/(6.4*sqrt(C))` means small task counts get massive K values (emergency recovery),
while large task counts get smaller K values (stability).

* docs: update zookeeper version (apache#18836)

* docs: update zookeeper version

* add link to zk release page

* Fix MSQ compaction state and native interval locking, add test coverage (apache#18950)

* MSQ compaction runner run test

* fix test

* fix test 2

* lock input interval

* test

* test coverage

* allowNonAlignedInterval and forceDropExisting

* fix test

* Update indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java

Co-authored-by: Lucas Capistrant <capistrant@users.noreply.github.com>

* Update indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java

Co-authored-by: Lucas Capistrant <capistrant@users.noreply.github.com>

* update

* style

* drop-existing

* Apply suggestion from @kfaraz

Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>

* format

* aligned

* build

* mis-aligned

* format

* test

* lock-interval

* lock

* test

* force drop existing, revert non-aligned, deprecated allowNonAlignedInterval

* revert THREE_HOUR

* revert format change

* test

* comment

* use-queue

* reduce test

* batchSegmentAllocation

---------

Co-authored-by: Lucas Capistrant <capistrant@users.noreply.github.com>
Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>

* Update assertj-core for CVE-2026-24400 (apache#18994)

Co-authored-by: Ashwin Tumma <ashwin.tumma@salesforce.com>

---------

Co-authored-by: Lucas Capistrant <capistrant@users.noreply.github.com>
Co-authored-by: Gian Merlino <gianmerlino@gmail.com>
Co-authored-by: Virushade <phuaguanwei99@gmail.com>
Co-authored-by: Clint Wylie <cwylie@apache.org>
Co-authored-by: Sasha Syrotenko <alexander.syrotenko@imply.io>
Co-authored-by: Vadim Ogievetsky <vadim@ogievetsky.com>
Co-authored-by: Andrei Pechkurov <37772591+puzpuzpuz@users.noreply.github.com>
Co-authored-by: jtuglu1 <jtuglu@netflix.com>
Co-authored-by: Cece Mei <yingqian.mei@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: 317brian <53799971+317brian@users.noreply.github.com>
Co-authored-by: mshahid6 <maryam.shahid1299@gmail.com>
Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>
Co-authored-by: aho135 <andrewho135@gmail.com>
Co-authored-by: Abhishek Radhakrishnan <abhishek.rb19@gmail.com>
Co-authored-by: Ashwin Tumma <ashwin.tumma23@gmail.com>
Co-authored-by: Ashwin Tumma <ashwin.tumma@salesforce.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clintropolis clintropolis clintropolis approved these changes

@kgyrtkirk kgyrtkirk Awaiting requested review from kgyrtkirk

Assignees

No one assigned

Labels

Area - Querying

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gianm @clintropolis