Support AT semantics for window measures#23
Conversation
![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: 9684977769
ℹ️ 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".
yardstick-rs/src/sql/measures.rs
Outdated
| /// Returns true if expression appears to use a SQL window clause. | ||
| fn is_window_expression(expr: &str) -> bool { | ||
| let expr_upper = expr.to_uppercase(); | ||
| expr_upper.contains(" OVER(") || expr_upper.contains(" OVER (") |
There was a problem hiding this comment.
is_window_expression only checks for the literal substrings " OVER(" and " OVER (", so valid window syntax such as line breaks/tabs before OVER or named windows like OVER w is not detected. In those cases extract_measures_from_sql will classify the measure as decomposable and replace it with NULL AS <measure>, which silently breaks both direct measure queries and the new AT (...) window handling for otherwise valid SQL formatting.
Useful? React with 👍 / 👎.
yardstick-rs/src/sql/measures.rs
Outdated
| .derived_expr | ||
| .clone() | ||
| .unwrap_or_else(|| resolved.expression.clone()); | ||
| let is_window_measure = is_window_expression(&resolved.expression); |
There was a problem hiding this comment.
This branch decides whether to apply window-specific single-value enforcement using resolved.expression, but execution uses expression_for_eval (which can be an expanded derived expression). For derived measures that expand into a window expression, the code takes the non-window path and emits a scalar subquery without the distinct-value guard, which can fail at runtime (multi-row scalar subquery) or return incorrect AT results instead of the intended window ambiguity error.
Useful? React with 👍 / 👎.
1 participant
Summary
AGGREGATE(window_measure) AT (...)support using context-first recomputationATand throw a clear error when multiple distinct values existAGGREGATE(window_measure)behavior unchangedGROUP BYinsertion so windowOVER (ORDER BY ...)clauses are not misdetected as outerORDER BYATwindow-measure casesValidation
make release./build/release/test/unittest test/sql/measures.test