Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50ce5694c3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for item in items { | ||
| let item_upper = item.to_uppercase(); | ||
| if item_upper.contains("AGGREGATE(") { | ||
| if has_aggregate_function(&item) { |
There was a problem hiding this comment.
Avoid skipping non-AGGREGATE functions that end with AGGREGATE
This now calls has_aggregate_function to decide whether to drop a SELECT item from implicit GROUP BY. That helper scans for any occurrence of AGGREGATE and returns true if the following non-whitespace char is (, so an expression like TOTAL_AGGREGATE (revenue) or a UDF named myaggregate will be treated as the special AGGREGATE function and excluded from grouping. In those cases, the dimension column is silently removed, changing query semantics. Consider matching AGGREGATE as a standalone function name (e.g., word boundary or full token) before skipping.
Useful? React with 👍 / 👎.
Fixes AGGREGATE detection in implicit GROUP BY logic so whitespace before parentheses is accepted. Adds coverage for spaced AGGREGATE calls and AT modifiers.