Skip to content

[CALCITE-7647] Support SELECT * in GROUP BY ALL and ORDER BY ALL#5089

Open
tisyabhatia wants to merge 1 commit into
apache:mainfrom
tisyabhatia:group-by-order-by-all-select-star
Open

[CALCITE-7647] Support SELECT * in GROUP BY ALL and ORDER BY ALL#5089
tisyabhatia wants to merge 1 commit into
apache:mainfrom
tisyabhatia:group-by-order-by-all-select-star

Conversation

@tisyabhatia

@tisyabhatia tisyabhatia commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Jira Link

CALCITE-7647

Changes Proposed

Expand a bare or qualified star in the SELECT clause to its underlying columns when rewriting GROUP BY ALL / ORDER BY ALL, so those clauses no longer reject SELECT *. Star items are expanded via the existing column-enumeration path (matching normal SELECT * semantics), before the placeholder is replaced with the concrete keys.

Expand a bare or qualified star in the SELECT clause to its underlying columns when rewriting GROUP BY ALL / ORDER BY ALL, so those clauses no longer reject SELECT *. Star items are expanded via the existing column-enumeration path (matching normal SELECT * semantics, including NATURAL JOIN coalescing), before the placeholder is replaced with the concrete keys.
* 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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does this differ from expand star?
Can this be avoided by doing the rewriteOrderByAll after the star expansion?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expandStarForAllRewrite just runs the normal star expansion on the *. I don't call the public expandStar because it expands the whole SELECT list before GROUP BY ALL is replaced and that makes the real columns look "not grouped."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you give an example? I don't quite understand either.

@tisyabhatia tisyabhatia Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 reports 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.

@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown


# [CALCITE-7647] GROUP BY ALL expands SELECT * to every underlying column;
# the star columns become grouping keys and the aggregate is excluded
select *, count(*) as c from (values (1, 'a'), (1, 'a'), (2, 'b')) as t(x, y)

@xuzifu666 xuzifu666 Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to know if this has been verified through specific database testing which @mihaibudiu also metioned in jira.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants