Traverse native query scopes and aggregate expressions - #48
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: 8ac5e02286
ℹ️ 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".
| : names.size() == 2 && scope.alias_is_local && !local_alias.empty() && | ||
| names[0] == Identifier(local_alias); |
There was a problem hiding this comment.
When an unaliased measure view is schema-qualified, DuckDB represents a filter such as main.sales_v.year = 2023 with three column-name components. This branch only recognizes exactly two components, so the reference remains bound to the outer result row instead of being rewritten to _inner.year; during default/VISIBLE recomputation, SELECT AGGREGATE(revenue) FROM main.sales_v WHERE main.sales_v.year = 2023 can therefore sum all base rows rather than only 2023. Recognize the relation component of multipart-qualified references as local as well.
Useful? React with 👍 / 👎.
| if (names.size() > 1 && scope.nested) { | ||
| auto qualifier = StringUtil::Lower(YsName(names.front())); | ||
| belongs_to_outer = outer_qualifiers.count(qualifier) && !scope.shadowed.count(qualifier); |
There was a problem hiding this comment.
For a correlated projection such as SELECT (SELECT main.sales_v.year), revenue FROM main.sales_v, the column reference has at least three components, but this comparison uses names.front() (main) while NativeRelationQualifiers records the relation name (sales_v). The outer dependency is consequently omitted from subquery_dimensions; dimension extraction then adds no corresponding GROUP BY, causing the expanded aggregate query to fail binding or collapse groups incorrectly. Match the table/relation portion of multipart-qualified references rather than only the first component.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
1 participant
Use DuckDB's native parse tree to lower CTE bodies, subqueries, and set operands in their own scopes, and discover aggregate calls through expression nodes. Preserve correlated filters, grouping dependencies in subquery projections, and implicit measure output names.
Native filter rewrite failures now propagate instead of silently retrying compatibility lowering. DuckDB 1.5.5 keeps its existing parser path. Repair the 12 stale Rust assertions to match current base-row recomputation and measure placeholders, and add native SQL regressions in both parser modes.