feat(sql-parser): support two-layer nested aggregation queries by milindsrivastava1997 · Pull Request #892 · ProjectASAP/ASAPQuery

milindsrivastava1997 · 2026-10-14T23:38:38Z

Summary

Supports two-layer SQL queries: an aggregation over a subquery that aggregates the table.

SELECT job, SUM(v) FROM (
  SELECT job, instance, SUM(value) AS v FROM data
  WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW()
  GROUP BY job, instance
) GROUP BY job

This uses the same approach as PromQL's nested aggregations. The inner layer is planned and served like any single-level query, and the outer layer runs as a query-time aggregation over its rows.

sql-parser: a new method, SQLPatternMatcher::nested_query_to_pattern, accepts an outer SUM/COUNT/AVG/MIN/MAX/QUANTILE over the inner aggregate's alias. The outer GROUP BY must be a subset of the inner GROUP BY; with no outer GROUP BY, all rows are aggregated. query_info_to_pattern still rejects nested queries. The parser now rejects clauses it can't represent instead of dropping them: an outer WHERE, HAVING, and inner ORDER BY / LIMIT.
sql-parser, all layers including flat queries: joins, extra FROM entries, QUALIFY, CONNECT BY and value-table modes are rejected rather than ignored. CTEs (WITH) are now rejected outright. The old CTE-to-subquery rewrite bound FROM to the first CTE whatever name it referenced, and dropped its joins. Every CTE query used to come out nested, and nested queries were rejected, so no query that was supported before is lost.
planner: the inner subquery is stored as planned_subquery and planned through the existing single-level path. The outer layer is emitted as one query_time_aggregations stage. The new QueryTimeAggregationOperator::from_name holds the name-to-operator mapping, and the PromQL and SQL planners both use it.
query-engine: runs the incoming query's inner layer with the nested query's config, then applies apply_instant_pipeline. If the config has no query-time aggregations, the nested query is not served. Serving it would execute only the inner layer and silently drop the outer aggregation.

Scope is two layers only, with no top-k. Three or more layers, #818, #820 and #821 are unchanged. Outer ORDER BY / LIMIT is rejected and tracked in #891. The --planner milp path still rejects nested queries.

SQL query support suite

cargo test -p query_engine_rust --test sql_query_support: 105 → 115 of 228 cases supported. All 228 cases match their expected outcome.

Suite	Before	After
`nested-aggregations`	0 / 91	8 / 91
`basic`	21 / 30	23 / 30
others (unchanged)	84 / 107	84 / 107

The new supported cases are the 8 non-top-k one-stage-* cases in nested-aggregations and the 2 nested cases in basic (nested_sum_of_sum, nested_avg_of_quantile).

Tests

sql_utilities: the nested matcher accepts each allowed outer aggregation and rejects each unsupported shape. The parser rejects each clause it can't represent, on both layers and on flat queries, and rejects CTEs.
asap_types: every outer aggregation the SQL matcher accepts maps to an operator.
planner: stage operator, grouping and quantile parameter per aggregation. The no-GROUP BY case uses all grouping. A nested query's streaming plan equals its inner layer's.
query-engine: executes against stored sums and checks the result values for outer SUM, QUANTILE and no-GROUP BY SUM. A config without stages is not served.

Closes #819.

🤖 Generated with Claude Code

Add SQLPatternMatcher::nested_query_to_pattern, which accepts an outer SUM/COUNT/AVG/MIN/MAX/QUANTILE over the inner aggregate's alias, grouped by a subset of the inner GROUP BY, and returns the inner layer's pattern. query_info_to_pattern still rejects nested queries. The parser now rejects clauses on either layer it cannot represent (outer WHERE, HAVING, inner ORDER BY / LIMIT) instead of dropping them. Part of #819. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ime aggregation A nested SQL query is planned through its inner layer, like any single-level query, and its outer layer is emitted as a query-time aggregation stage, mirroring PromQL's nested_aggregation_plan. Part of #819. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ation A nested SQL query is executed through its inner layer, using the nested query's config, and the config's query-time aggregations are applied to the result as its outer layer. A nested query whose config has no query-time aggregations is not served, since running the inner layer alone would drop the outer aggregation. The 8 non-topk one-stage nested-aggregations cases and the two nested cases in basic.yaml are now supported. Closes #819. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

milindsrivastava1997

Code review (unverified findings, so please double-check).

Bug outside the diff, sqlpattern_parser.rs:163: a CTE can be bound to the wrong table. cte_to_subquery always uses cte_tables[0] and overwrites the outer FROM whatever name it references. Every CTE query becomes nested, so this was unreachable until this PR started serving nested queries. WITH a AS (... SUM ...), b AS (... MAX ...) SELECT job, SUM(v) FROM b GROUP BY job is now served using a, which silently gives the wrong result.

Inline comments cover the rest. The first three (the CTE planning failure, the dropped outer joins, and this one) should block merge.

milindsrivastava1997 · 2026-10-15T00:07:47Z

+}
+
+/// The SQL text of the subquery in the outer layer's FROM.
+fn inner_query_sql(stmts: &[Statement]) -> Option<String> {


Bug: a CTE-form nested query fails planning for the whole workload. SQLPatternParser rewrites WITH t AS (...) SELECT ... FROM t into a FROM-subquery, so qdata.subquery is Some and the query matches as nested. But inner_query_sql reads the raw, un-rewritten statements, where FROM is a plain table name, so it returns SqlParse("nested query has no subquery in FROM"). generate_sql_plan propagates that with ? (generator.rs:99), so every query in the workload fails planning.

Repro: WITH t AS (SELECT job, instance, SUM(value) AS v FROM data WHERE time BETWEEN ... GROUP BY job, instance) SELECT job, SUM(v) FROM t GROUP BY job

milindsrivastava1997 · 2026-10-15T00:07:47Z

+            }
+
            // Parse subquery
            let subquery = match &select.from[0].relation {


Bug: joins on the outer query are silently dropped. The nested branch only looks at select.from[0].relation and never rejects select.from[0].joins or additional FROM entries. SELECT job, SUM(v) FROM (<inner>) t JOIN other o ON t.job = o.job GROUP BY job used to be rejected with NestedQueryUnsupported; now it's served with the join ignored, which gives a wrong but valid-looking answer. This contradicts the PR's claim that unrepresentable clauses are rejected on both layers.

milindsrivastava1997 · 2026-10-15T00:07:47Z

    }

+    /// SELECT clauses that SQLQueryData cannot represent.
+    fn has_unsupported_clauses(select: &Select) -> bool {


Bug: QUALIFY is silently dropped. has_unsupported_clauses doesn't check select.qualify. Now that this helper guards both layers, ... GROUP BY job, instance QUALIFY v > 10 in the inner query is served with the filter ignored, so the outer aggregate includes rows that should have been filtered out.

milindsrivastava1997 · 2026-10-15T00:07:47Z

+            );
+            return None;
+        }
+        let inner = query_data.subquery.as_deref()?;


Risk: the planner's inner query and the engine's inner query are never reconciled. The engine rebuilds the inner layer from the incoming query_data and ignores config.planned_subquery. The planner sizes aggregations from the ClickHouse-dialect inner SQL (re-serialized via to_string), while the engine re-parses with GenericDialect. If the two differ in round-tripping or dialect (e.g. parametric quantile(0.95)(v)), the config's aggregation_keys get applied to a differently shaped inner query with no error. Consider checking the two against each other, or using planned_subquery directly.

Leaving this as is. The engine doesn't look up the config by planned_subquery: find_query_config_sql matches the incoming query against config.query, the original nested text, and matches_sql_pattern compares both layers structurally (metric, aggregation, labels, time range). So the config's aggregation keys only ever serve an inner layer that has the same shape as the one the planner planned. planned_subquery is informational, as it is for flat queries. I also checked the round trip that planned_subquery goes through (ClickHouse dialect, then to_string, then planning) for parametric quantile(0.95)(value), half-open ranges and CAST(... AS DateTime). Each one re-serializes with the same meaning.

milindsrivastava1997 · 2026-10-15T00:07:47Z

+            (output_labels, result)
+        } else {
+            let QueryResult::Vector(InstantVector { values, timestamp }) = result else {
+                error!("SQL query-time aggregations need an instant vector");


Bug: a global outer aggregate over an empty inner result returns zero rows. SQL requires exactly one row here (0 for SUM/COUNT, NULL for the others). SELECT SUM(v) FROM (<inner> GROUP BY job, instance) over an empty window: ClickHouse returns 0, but apply_instant_pipeline with All grouping over no elements returns nothing, so the results won't match in the equivalence tests.

This isn't new behavior. An empty window has no local data, so execute_context returns None and the query falls through to the fallback, which returns the SQL-correct single row. Only a window that exists but holds zero rows comes back empty, and the flat global aggregates from #826 behave the same way in that case. So this would be a shared fix for flat and nested queries, outside this PR.

milindsrivastava1997 · 2026-10-15T00:07:47Z

 }

+/// Aggregations an outer layer can apply to its inner layer's rows.
+const QUERY_TIME_AGGREGATIONS: [&str; 6] = ["SUM", "COUNT", "AVG", "MIN", "MAX", "QUANTILE"];


Cleanup: the allowed outer aggregations are listed twice. They're in QUERY_TIME_AGGREGATIONS here and again in the name-to-operator match in planner/sql.rs (sql_query_time_aggregation). If someone adds an operator to one list but not the other, the matcher accepts queries the planner then rejects, or the reverse. A single name-to-operator mapping in asap_types would remove the duplicate.

milindsrivastava1997 · 2026-10-15T00:07:47Z

+        query_evaluation_time: f64,
+    ) -> Result<Option<NestedAggregationPlan>, ControllerError> {
+        let schema = build_sql_schema(&self.table_definitions);
+        let (stmts, qdata) =


Cleanup: queries get parsed twice. nested_aggregation_plan builds the schema and fully parses every query, then get_streaming_aggregation_configs builds the schema and parses the same query again for every single-level query. Returning the parsed qdata/stmts from one call, or deciding nested vs. single-level in one place, would avoid the repeated parse and schema clone.

Leaving this as is. The extra parse only happens in the offline planner, and costs a few microseconds per query. Sharing the parsed qdata would mean changing get_streaming_aggregation_configs to take pre-parsed data, which isn't worth it for that cost.

milindsrivastava1997 · 2026-10-15T00:07:47Z

+) -> Result<QueryTimeAggregation, ControllerError> {
+    let info = &outer.aggregation_info;
+    let operator = match info.get_name() {
+        "SUM" => QueryTimeAggregationOperator::Sum,


Cleanup: this duplicates promql.rs::aggregation_stage. sql_query_time_aggregation re-implements the operator mapping, the All/By grouping construction and the quantile-parameter handling. The two copies can drift apart; for example, PromQL checks the phi range inline while SQL relies on validate(). A shared helper taking (operator name, labels, param) would serve both.

milindsrivastava1997 · 2026-10-15T00:07:47Z

+                        // Query-level clauses of the inner layer have no place
+                        // in its SQLQueryData, so they would be dropped.
+                        if subquery.with.is_some()
+                            || subquery.order_by.is_some()


Nit: misleading error message. A WITH inside the subquery is rejected with "ORDER BY / LIMIT inside a subquery is not supported", which names a clause the user's query doesn't have and makes the rejection hard to debug. Give the WITH case its own message.

cte_to_subquery bound FROM to the first CTE whatever name it referenced, and replaced FROM wholesale, dropping its joins. Every CTE query was nested and so rejected until nested queries became supported; reject WITH outright instead. Joins, extra FROM entries, QUALIFY, CONNECT BY and value-table modes are now rejected on every layer, flat queries included, rather than silently ignored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Add QueryTimeAggregationOperator::from_name and use it in both the PromQL and SQL planners. A test checks that every outer aggregation the SQL matcher accepts maps to an operator, since sql_utilities cannot depend on asap_types to share the list itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

milindsrivastava1997 and others added 3 commits October 14, 2026 19:25

milindsrivastava1997 linked an issue Oct 14, 2026 that may be closed by this pull request

sql: two-layer nested aggregation (subquery in FROM) is rejected #819

Closed

milindsrivastava1997 commented Oct 15, 2026

View reviewed changes

milindsrivastava1997 and others added 2 commits October 14, 2026 20:10

milindsrivastava1997 merged commit 86dd6d2 into main Oct 15, 2026
9 checks passed

milindsrivastava1997 deleted the 819-sql-two-layer-nested-aggregation-subquery-in-from-is-rejected branch October 15, 2026 00:20

Conversation

milindsrivastava1997 commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

SQL query support suite

Tests

Uh oh!

milindsrivastava1997 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

milindsrivastava1997 commented Oct 14, 2026 •

edited

Loading