[ 
https://issues.apache.org/jira/browse/CALCITE-3893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17080233#comment-17080233
 ] 

Feng Zhu commented on CALCITE-3893:
-----------------------------------

{quote}If someone can back up the "a bit hard to implement", I will listen. One 
data point against that argument is that when [~vlsi] and [~donnyzone] 
implemented CALCITE-1824, {{EnumerableAggregate.implement}} did the right thing 
first time.

But I think it is more likely that people think the plan just looks a bit weird.
{quote}
Yes, current implementor ({{EnumerableAggregate.implement)}} can handle the 
plan correctly (as expected).

But if we allow this pattern, we need to fix the logic of RelToSqlConverter.
{quote}If you don't like it, or your implementor can't handle it, fell free to 
write a rule to transform it to
{quote}
The runtime behevior of P1 and P2 are different.
{noformat}
P1:
------------------------------------------------------------
LogicalAggregate(group=[{0}], groups=[[{}]], C=[COUNT()])

SQL: select null, count(*) as c from emp


P2
------------------------------------------------------------
LogicalProject(null AS DEPTNO, $1)
  LogicalAggregate(group=[{0}], groups=[[{}]], C=[COUNT()])

SQL: select null, c from (select deptno, count(*) as c from emp)
{noformat}
 

> SQL with GROUP_ID may generate wrong plan
> -----------------------------------------
>
>                 Key: CALCITE-3893
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3893
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.22.0
>            Reporter: Shuo Cheng
>            Assignee: Feng Zhu
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.23.0
>
>          Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
> Consider the following SQL:
> {code:java}
> select 
>   deptno, group_id() as g, count(*) as c
> from emp 
> group by grouping sets (deptno, (), ())
> {code}
> the plan after SqlToRel is:
> {code:java}
> LogicalUnion(all=[true])
>   LogicalProject(DEPTNO=[$0], G=[0:BIGINT], C=[$1])
>     LogicalAggregate(group=[{0}], groups=[[{0}, {}]], C=[COUNT()])
>       LogicalProject(DEPTNO=[$7])
>         LogicalTableScan(table=[[CATALOG, SALES, EMP]])
>   LogicalProject(DEPTNO=[$0], G=[1:BIGINT], C=[$1])
>     LogicalAggregate(group=[{0}], groups=[[{}]], C=[COUNT()])
>       LogicalProject(DEPTNO=[$7])
>         LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> {code}
> I'm afraid there's some semantic problems here. As `groups` of the second 
> Aggregate is empty, then what is `$0` meaning in the Project above it. Maybe 
> that we want is:
> {code:java}
> LogicalUnion(all=[true])
>   LogicalProject(DEPTNO=[$0], G=[0:BIGINT], C=[$1])
>     LogicalAggregate(group=[{0}], groups=[[{0}, {}]], C=[COUNT()])
>       LogicalProject(DEPTNO=[$7])
>         LogicalTableScan(table=[[CATALOG, SALES, EMP]])
>   LogicalProject(DEPTNO=[$0], G=[1:BIGINT], C=[$1])
>     LogicalAggregate(group=[{0}], groups=[[{0}]], C=[COUNT()])
>       LogicalProject(DEPTNO=[null])
>         LogicalTableScan(table=[[CATALOG, SALES, EMP]]){code}
> I noticed this is introduced by CALCITE-1824, cc [~donnyzone], 
> [~vladimirsitnikov].



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to