Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 680e25adc3
ℹ️ 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".
| outer_cols | ||
| .iter() | ||
| .filter(|col| view_set.contains(&normalize_group_by_col(col))) | ||
| .cloned() |
There was a problem hiding this comment.
Preserve alias-based group-by dims when filtering
This filter keeps only outer GROUP BY columns whose normalized text matches the view’s GROUP BY list. When a view groups by an expression but the outer query groups by the expression’s alias (e.g., view GROUP BY date_trunc(...) but outer GROUP BY month), normalize_group_by_col won’t consider those equal, so the alias column is dropped. That makes AT correlation ignore a real dimension and can collapse results to a grand total (e.g., AT (ALL region) ends up with no remaining dims). This happens specifically when the view uses an expression in its GROUP BY and the outer query uses the alias instead of repeating the expression.
Useful? React with 👍 / 👎.
Filter AT correlations to dimensions in the measure's source view to avoid cross-table joins. Adds a regression test covering joins with extra dimensions.