Skip to content

feat(sql-utilities): support multiple SELECT projections with one aggregate and add SQL ORDER BY and LIMIT support #360

Closed
akanksha-akkihal wants to merge 3 commits into
mainfrom
support-for-SQLQueries-Netflow
Closed

feat(sql-utilities): support multiple SELECT projections with one aggregate and add SQL ORDER BY and LIMIT support #360
akanksha-akkihal wants to merge 3 commits into
mainfrom
support-for-SQLQueries-Netflow

Conversation

@akanksha-akkihal

Copy link
Copy Markdown
Collaborator

feat(sql-utilities): support multiple SELECT projections with one aggregate

Walk the projection list and accept one aggregate function alongside any
number of plain column references, instead of requiring projection.len()==1.
The natural form "SELECT g1, g2, agg(v) FROM t GROUP BY g1, g2" now parses
where it previously fell through to destination=none_unsupported.

Each non-aggregate identifier in SELECT must also appear in GROUP BY,
matching standard SQL. Without this check the parser would silently drop
SELECT identifiers absent from GROUP BY (e.g. SELECT srcip, SUM(v) ...
GROUP BY proto would treat the query as if srcip weren't requested).

Multiple aggregates, computed expressions, literals, and wildcards remain
rejected since the structural pattern model only tracks one statistic.
Column-extraction logic for QUANTILE / PERCENTILE / generic aggregations
is unchanged.

Adds 8 tests: group-cols-then-aggregate, aggregate-first ordering,
ClickHouse parametric quantile in multi-projection form, structural
matching between single- and multi-projection equivalents, SELECT subset
of GROUP BY, and rejection of non-GROUP-BY identifiers, two aggregates,
and arbitrary expressions.

@akanksha-akkihal

Copy link
Copy Markdown
Collaborator Author

feat(sql-utilities, query-engine): add SQL ORDER BY and LIMIT support for grouped aggregate queries

Capture ORDER BY items, LIMIT N, and the aggregate alias during the
initial SQL parse, store them as new fields on SQLQueryData, and apply
them to the result vector after execute_query_pipeline returns. The
top-N form "SELECT g1, g2, agg(v) AS p99 ... ORDER BY p99 DESC LIMIT 10"
now parses and executes where it previously fell through to
destination=none_unsupported.

Each ORDER BY column must reference either the aggregate alias or a
GROUP BY key, validated at parse time. Without this check the engine
would silently return unsorted results when a typo'd or unknown column
appears in ORDER BY. Multi-column ORDER BY with per-column ASC/DESC is
supported via a chained comparator; positional refs, expressions, OFFSET,
LIMIT BY, MySQL "LIMIT a, b", WITH FILL, NULLS FIRST/LAST, and ClickHouse
"ORDER BY ALL" remain rejected since they have no obvious mapping to the
precompute model.

Sort and truncate live in a new SqlPostProcessing side-channel returned
from build_sql_query_pieces alongside QueryExecutionContext, so the
shared context stays untouched and PromQL/Elastic engines are unaffected.
matches_sql_pattern ignores the three new fields so registered templates
without ORDER BY/LIMIT still match incoming queries that add them — they
are presentational, not structural. SqlPostProcessing::apply short-circuits
when both order_by and limit are unset, so non-SQL paths pay zero cost.

Adds 16 tests: aggregate-alias-DESC-with-LIMIT (the user-facing top-N
case), default-ASC, multi-column mixed directions, ORDER BY on a GROUP BY
column, LIMIT-only, multi-key tiebreak, NaN safety, the matches_sql_pattern
contract, no-op SqlPostProcessing fast path, end-to-end through
SqlPostProcessing::apply, and rejection of unknown column, non-identifier
expression, and OFFSET. TDD-validated by stashing the implementation and
confirming the new tests fail to compile against the original data model.

@akanksha-akkihal akanksha-akkihal changed the title feat(sql-utilities): support multiple SELECT projections with one aggregate feat(sql-utilities): support multiple SELECT projections with one aggregate and add SQL ORDER BY and LIMIT support May 23, 2026
@akanksha-akkihal akanksha-akkihal deleted the support-for-SQLQueries-Netflow branch May 24, 2026 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant