feat(sql): lower explicit temporal aggregates - #369
Open
zzylol wants to merge 2 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
ClickHouse SQL could express ordinary relational aggregates, but it could not describe PromQL-equivalent counter rate or counter increase semantics to ASAPPlanner. Rewriting these operations as
sumormaxchanges their meaning. Applying either reducer after multiple counter series have been combined is also incorrect.What
Adds two explicit SQL planning functions:
asap_rate(value, timestamp, window_ms)asap_increase(value, timestamp, window_ms)They lower to the existing canonical
AggIntent::RateandIncreaseforms. NoQueryExpr,AggIntent, SDS, or post-ASAP wire type changes.asap_lastis deliberately unsupported: the current physical planner/runtime has no executable LastOverTime SDS path, so the SQL frontend fails closed rather than advertising an acceleration path it cannot execute.How
The SQL function catalog registers fixed-arity pass-through UDAFs. The SQL frontend validates that:
valueis a numeric, non-time column;timestampis the input schema's declared time index;window_msis a positive integer literal;GROUP BYexactly matches a declared series identity.The minimal series-identity contract uses existing
Schema::unique_keyswithout changing its meaning: a row-unique key must include the declared time-index column; removing the time index yields the series key. SQLGROUP BYmust match that key by resolved column ID. This rejects absent/partial identities, grouping by value/time, duplicate qualified/unqualified references to the same column, and schemas whose unique keys do not include time.After validation, the frontend projects explicit SQL columns into
(ts, value, series labels...), wraps them in the existingTimeRange, and emits the existingReduction::PerEntityaggregate. Cross-series reduction composes in an outer query, such asSELECT sum(v) FROM (...).Before this PR
SELECT service, asap_rate(value, ts, 300000) ... GROUP BY servicefailed as an unknown function. Ordinary SQL aggregate rewrites could not preserve reset-aware rate semantics or prove that samples from distinct counters stayed separate.After this PR
Given catalog metadata
time_index = tsandunique_keys = [[ts, service]], the same query lowers as:ASAP-aware physical mapping produces the existing
ExactAggregate(Rate)summary.asap_increasesimilarly producesExactAggregate(Increase). Project, source Filter, derived-table Filter, and outer relational Aggregate nodes remain in the plan.Evidence
The end-to-end integration test feeds both functions through ClickHouse SQL parsing, canonical lowering, and ASAP-aware mapping and observes the matching shared exact physical summary family with
Reduction::PerEntityover the retained 300-second range.Verification
cargo test -p asap-sql-function-catalog;cargo test -p asap-frontend-sql.cargo test -p asap-integration-tests --test sql_to_post_asap.cargo fmt --all -- --check;cargo clippy -p asap-frontend-sql -p asap-sql-function-catalog -p asap-integration-tests --tests -- -D warnings;git diff --check.New tests cover rate/increase lowering, complete and partial multi-series identities, empty grouping, qualified references resolved by column identity, grouping by value/time, duplicate resolved grouping columns, invalid timestamp/window arguments, mixed reducers, Project/Filter/outer-Aggregate nesting, explicit Last failure, and both functions' physical summary selection.
Architectural decisions
The bridge uses existing canonical nodes and physical families. SQL-only syntax recognition, identity proof, and column adaptation remain inside the SQL frontend.
The functions are explicit because standard ClickHouse SQL has no aggregate with PromQL-compatible reset-aware/extrapolated rate semantics.
argMax(value, timestamp)remains the existing row-selector extension: it has no explicit range and is not equivalent tolast_over_time.Limitations and follow-up
asap_*unchanged.Human review — do not complete with an agent