[CALCITE-7679] RelToSqlConverter generates GROUP BY literals for dial… - #5130
[CALCITE-7679] RelToSqlConverter generates GROUP BY literals for dial…#5130ehds wants to merge 1 commit into
Conversation
0a2caeb to
c9b21de
Compare
| public final Result visitRoot(RelNode r) { | ||
| List<RelOptRule> rules = new ArrayList<>(); | ||
| if (!this.dialect.supportsGroupByLiteral()) { | ||
| rules.add(CoreRules.PROJECT_MERGE); |
There was a problem hiding this comment.
It's not obvious to me why this solves the problem. Can you explain?
There was a problem hiding this comment.
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_idIt 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.
There was a problem hiding this comment.
This works for this particular plan, but why does it do in general what you need?
There was a problem hiding this comment.
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.idAnd this approach is similar to the fixes used for other cases,such as CALCITE-7655, CALCITE-4491
There was a problem hiding this comment.
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
c9b21de to
3cd7d34
Compare
|



…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_MERGEandPROJECT_REMOVE.