[CALCITE-3662] Generate wrong SQL when plan contains Project(Sort(Aggregate)) and aggregate field has no alias

[helloppx]

https://issues.apache.org/jira/browse/CALCITE-3662

[jinxing64]

How about add a test in RelToSqlConverterTest ?

[helloppx]

How about add a test in RelToSqlConverterTest ?

I did some research. If we want to reproduce this JIRA case, we must use SqlToRelConverter to convert SQL to REL and then use RelToSqlConverter to generate SQL. If we move this case to RelToSqlConverterTest, It is not easy to use SqlToRelTestBase#createTester()#convertSqlToRel(sql) method for this case.

[yanlin-Lynn]

I add a test case in RelToSqlConverterTest, it seems to produce a correct sql

@Test public void testGroupByWithLimitPlan() {
    String query = "SELECT sum(\"salary\") FROM \"employee\" group by \"gender\" limit 10";
    sql(query)
        .ok("SELECT SUM(\"salary\")\n"
            + "FROM \"foodmart\".\"employee\"\n"
            + "GROUP BY \"gender\"\n"
            + "FETCH NEXT 10 ROWS ONLY");
  }

[helloppx]

I add a test case in RelToSqlConverterTest, it seems to produce a correct sql

@Test public void testGroupByWithLimitPlan() {
    String query = "SELECT sum(\"salary\") FROM \"employee\" group by \"gender\" limit 10";
    sql(query)
        .ok("SELECT SUM(\"salary\")\n"
            + "FROM \"foodmart\".\"employee\"\n"
            + "GROUP BY \"gender\"\n"
            + "FETCH NEXT 10 ROWS ONLY");
  }

Hi @yanlin-Lynn @danny0405 If we want to reproduce this JIRA case we need to make sure Sort is input of Project [1]. If we move the case to RelToSqlConverterTest, it will produce a plan [2] so we cannot reproduce the JIRA case.

[1]

JdbcToEnumerableConverter
  JdbcProject(EXPR$0=[$1])
    JdbcSort(fetch=[10])
      JdbcAggregate(group=[{1}], EXPR$0=[SUM($5)])
        JdbcTableScan(table=[[SCOTT, EMP]])

[2]

LogicalSort(fetch=[10])
  LogicalProject(EXPR$0=[$1])
    LogicalAggregate(group=[{0}], EXPR$0=[SUM($1)])
      LogicalProject(brand_name=[$2], net_weight=[$7])
        JdbcTableScan(table=[[foodmart, product]])

[yanlin-Lynn]

Currently, I'm using RelToSqlConverter in my project, so I spent some time on this. I tried to fix this in buildAggregate, and it's affected 5 test cases in RelToSqlConverterTest. Here is the prototype of my fix, https://github.com/yanlin-Lynn/calcite/tree/CALCITE-3662, hope it's of some use to you.

[danny0405]

Can you fix the code conflict ?

[yanlin-Lynn]

We can reproduce the case in RelToSqlConverterTest, you can apply SortProjectTransposeRule.INSTANCE rule to make sure Sort transpose Project

[helloppx]

Currently, I'm using RelToSqlConverter in my project, so I spent some time on this. I tried to fix this in buildAggregate, and it's affected 5 test cases in RelToSqlConverterTest. Here is the prototype of my fix, https://github.com/yanlin-Lynn/calcite/tree/CALCITE-3662, hope it's of some use to you.

Thank you @yanlin-Lynn , I have moved the test case. :D