tisyabhatia commented on code in PR #5089:
URL: https://github.com/apache/calcite/pull/5089#discussion_r3553856414
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -5256,6 +5256,24 @@ protected void validateOrderList(SqlSelect select) {
}
}
+ /** Expands a single "*" or "t.*" select item into its underlying columns,
+ * for a GROUP BY ALL / ORDER BY ALL rewrite.
+ *
+ * <p>Calls the private {@code expandStar} core directly (not the public
+ * {@code expandStar(SqlNodeList, SqlSelect, boolean)} wrapper), with fresh
+ * collections: the wrapper would derive types over every select item and
+ * mark the expanded list, poisoning {@link AggregatingSelectScope}'s
+ * memoized grouping set with the not-yet-rewritten placeholder. The fresh
+ * {@code items}/{@code fields} must stay paired for NATURAL/USING index
+ * alignment. */
+ private List<SqlNode> expandStarForAllRewrite(SqlSelect select, SqlNode
starItem) {
Review Comment:
Let's say I had a query like `SELECT *, UPPER(ename) FROM emp GROUP BY ALL`.
When we first type-check a column, we figure out which columns we're grouping
by and cache that answer for good. So if I use the public `expandStar`, it
type-checks `UPPER(name)` before I've replaced `GROUP BY ALL` with the actual
columns. So internally, that stores "we don't have to group by anything." After
that, I can't change the that memory. We get the error "Expression `EMP.EMPNO`
is not being grouped."
It report `EMPNO` (first column), not `UPPER(ename)`.
Note: `SELECT * FROM emp GROUP BY ALL` works fine, but the extra
`UPPER(ename)` makes Calcite type-check early.
This helper only touches the `*` so nothing gets locked in before we've
decided the grouping columns.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
