Implement semantic SQL rewrite planner#174
Conversation
nicosuave
changed the title
nicosuave
force-pushed
the
semantic-sql-planner
branch
from
6a0e12b to
89dfba8
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89dfba82c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if dim_model_name == metric_model_name: | ||
| column_expr = f"{rollup_alias}.{self._quote_identifier(dim_name)}" | ||
| else: |
There was a problem hiding this comment.
When join-key preaggregation is selected and the query includes a local time dimension (for example orders.order_date__month), this branch always reads orders_rollup.order_date before applying DATE_TRUNC. Pre-aggregation tables store time dimensions as grain-specific columns (e.g. order_date_day), so the generated SQL references a non-existent column and fails at execution for these queries. This affects any join-key preaggregation route with time-grained local dimensions while use_preaggregations is enabled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf3cc4fe74
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| column_name = dim_name | ||
| if granularity and preagg.time_dimension == dim_name and preagg.granularity: | ||
| column_name = f"{dim_name}_{preagg.granularity}" | ||
| column_expr = f"{rollup_alias}.{self._quote_identifier(column_name)}" |
There was a problem hiding this comment.
When join-key preaggregation is selected and a query projects a local time dimension without an explicit __granularity suffix (for example orders.order_date), this code keeps column_name = dim_name and emits orders_rollup.order_date. Pre-aggregation tables only materialize the grain column (for example order_date_day), so these valid queries fail at execution with an unknown-column error whenever a time-grained local dimension is requested alongside remote dimensions.
Useful? React with 👍 / 👎.
| LEFT JOIN {remote_model.table} AS {remote_alias} | ||
| ON {" AND ".join(join_conditions)}{group_by_clause}{order_by_clause}{limit_clause}""" |
There was a problem hiding this comment.
This join always targets remote_model.table, which skips remote_model.sql when the remote model is defined by a SQL subquery/view. In that configuration, join-key preaggregation rewrites no longer apply the remote model’s semantic source logic (such as row filters or derived source columns), so optimized results can diverge from normal semantic queries and may include rows that should be excluded.
Useful? React with 👍 / 👎.
1 participant
Implements the semantic SQL rewrite planner and optimizer coverage in this PR: explainable rewrite plans, semantic island optimization, slicer/dropdown rewrites, additive totals/subtotals, Top-N variants, Superset-style wrapper pushdown, conditional aggregate pivots, safe time-grain expression recovery, set-operation branch optimization, and preaggregation route selection support.
Adds BI corpus accepted/rejected coverage, benchmark proof cases, and expanded explain/SQL-shape/execution-equivalence assertions. Also fixes related projection-order and cross-model subquery behavior found during validation.
Validated locally with ruff check, ruff format --check, and pytest -v.