Skip to content

[CALCITE-7679] RelToSqlConverter generates GROUP BY literals for dial… - #5130

Open
ehds wants to merge 1 commit into
apache:mainfrom
ehds:fix-group-by-iteral
Open

[CALCITE-7679] RelToSqlConverter generates GROUP BY literals for dial…#5130
ehds wants to merge 1 commit into
apache:mainfrom
ehds:fix-group-by-iteral

Conversation

@ehds

@ehds ehds commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

…ects that do not support them when the constant is hidden by nested Projects

Jira Link

CALCITE-7679

Changes Proposed

Before applying AggregateProjectConstantToDummyJoinRule, normalize nested projects using PROJECT_MERGE and PROJECT_REMOVE.

@ehds
ehds force-pushed the fix-group-by-iteral branch from 0a2caeb to c9b21de Compare July 28, 2026 15:36
public final Result visitRoot(RelNode r) {
List<RelOptRule> rules = new ArrayList<>();
if (!this.dialect.supportsGroupByLiteral()) {
rules.add(CoreRules.PROJECT_MERGE);

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.

It's not obvious to me why this solves the problem. Can you explain?

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.

I added a new UT to explain why we need theses two rules.

For this sql:

SELECT id, employee_id
FROM (
  SELECT id, employee_id
  FROM (
    SELECT employee_id, NULL AS id
    FROM employee
  ) AS t1
) AS t2
GROUP BY id, employee_id

It will be transformed to:

LogicalAggregate(group=[{0, 1}])
  LogicalProject(id=[$1], employee_id=[$0])
    LogicalProject(employee_id=[$0], id=[null:NULL])
      JdbcTableScan(table=[[foodmart, employee]])

AggregateProjectConstantToDummyJoinRule (added by CALCITE-4702) only matches the Agg->Project pattern, however, $1 and $0 are not literals, so the rule does not fire.

To fix it, we should remove unnecessary PROJECT operations by these two rules.

Or if you have a better suggestion, I'll follow up.

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.

This works for this particular plan, but why does it do in general what you need?

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.

Indeed, this current solution only handles particular cases.

I think a more general fix would be to wrap the input in a subquery when determining needNewSubQuery if any GROUP BY expression contains a literal.

LogicalAggregate(group=[{0, 1}])
  LogicalProject(id=[$1], employee_id=[$0])
    LogicalProject(employee_id=[$0], id=[null:NULL])
      JdbcTableScan(table=[[foodmart, employee]])

Translate this plan into the following SQL form:

SELECT employee_id, id 
   FROM (SELECT employee_id, null as id from foodmart.employee) as t
group by t.employee_id, t.id

And this approach is similar to the fixes used for other cases,such as CALCITE-7655, CALCITE-4491

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.

In general you should use the Jira to propose a design, and the PR to implement it.
This approach may work.

…ects that do not support them when the constant is hidden by nested Projects
@ehds
ehds force-pushed the fix-group-by-iteral branch from c9b21de to 3cd7d34 Compare July 29, 2026 03:08
@sonarqubecloud

Copy link
Copy Markdown

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.

2 participants