Skip to content

Domain uses AST analysis, not the SQL translator - #134

Open
austin-barrington wants to merge 1 commit into
refactor/code-healthfrom
refactor/query-analysis
Open

austin-barrington wants to merge 1 commit into
refactor/code-healthfrom
refactor/query-analysis

Conversation

@austin-barrington

Copy link
Copy Markdown
Member

Stacked on #133. Implements docs/superpowers/plans/2026-11-03-query-analysis-module.md.

timeseriesql::analysis now holds the pure InfluxQL AST facts the domain needs:

  • output column names;
  • aggregate detection;
  • the TOP/BOTTOM/SAMPLE selector descriptor;
  • the merge column names.

domain::rollup, domain::sharding::query_merge and domain::sharding::multi_selector now import from it instead of to_clickhouse. The translator re-exports the moved items, so the generated SQL is unchanged.

The move is pure: a reviewer compared every moved body line by line, and only visibility changed. New tests pin output names, that window transforms are not aggregates, and the merge column literals.

Gate: fmt, clippy -D warnings, 883 lib tests, 373 integration tests and 54 CLI tests pass.

timeseriesql::analysis holds the AST facts that hold whatever the backend:
select_output_field_name (with default_field_alias / influxql_name),
select_has_true_aggregate and the aggregate/window-transform predicates,
SelectorKind, MultiSelector, BUCKET_COL and TIME_NS_COL. to_clickhouse
re-exports them, so the translator is unchanged; domain::rollup,
sharding::query_merge and sharding::multi_selector import the analysis
module instead of to_clickhouse.
@cursor

cursor Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Pure refactor with line-for-line moves and translator re-exports; no intended behavior change to query translation or merge semantics.

Overview
Introduces timeseriesql::analysis as the home for backend-agnostic InfluxQL AST facts: output column naming (select_output_field_name, Influx-style aliases), true aggregate detection (excluding bare window transforms), TOP/BOTTOM/SAMPLE MultiSelector metadata, and shared merge column constants __bucket / __tns.

Domain code in rollup, sharding::query_merge, and sharding::multi_selector now imports these from analysis instead of to_clickhouse, so rollup and shard merge logic no longer depend on the ClickHouse translator.

The moved logic is lifted out of to_clickhouse/select, aggregates, and selectors; the translator re-exports the same public items and pulls in shared helpers (e.g. expr_contains_raw_transform) so generated SQL behavior should be unchanged. analysis adds unit tests for output names, window transforms vs aggregates, and merge column literals.

Reviewed by Cursor Bugbot for commit 88a2840. Configure here.

@cursor

cursor Bot commented Nov 4, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: bb52c930-66ad-405e-94ef-5139fa612dcb)

This branch has not been deployed

No deployments
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