From 2d0335a1fe8a253ee5cb96314a6c601de67b8834 Mon Sep 17 00:00:00 2001 From: Lei Jiang Date: Thu, 2 Jan 2020 16:13:26 +0800 Subject: [PATCH 1/8] [CALCITE-3662] Generate wrong SQL when plan contains Project(Sort(Aggregate)) and aggregate field has no alias (Lei Jiang) --- .../apache/calcite/rel/rel2sql/RelToSqlConverter.java | 9 +++++++-- .../java/org/apache/calcite/test/JdbcAdapterTest.java | 11 +++++++++++ .../java/org/apache/calcite/test/PigRelOpTest.java | 8 ++++---- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java index 46cd1efe4a38..8fe2abb31b64 100644 --- a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java +++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java @@ -227,8 +227,13 @@ public Result visit(Project e) { if (isStar(e.getChildExps(), e.getInput().getRowType(), e.getRowType())) { return x; } - final Builder builder = - x.builder(e, Clause.SELECT); + final Builder builder; + if (e.getInput() instanceof Sort) { + builder = x.builder(e); + builder.clauses.add(Clause.SELECT); + } else { + builder = x.builder(e, Clause.SELECT); + } final List selectList = new ArrayList<>(); for (RexNode ref : e.getChildExps()) { SqlNode sqlExpr = builder.context.toSql(null, ref); diff --git a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java index 361aa4a301d4..7d09fca60a84 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java @@ -177,6 +177,17 @@ public class JdbcAdapterTest { + "ON \"t\".\"DEPTNO\" = \"t0\".\"DEPTNO\""); } + @Test public void testGroupByWithLimitPlan() { + CalciteAssert.model(JdbcTest.SCOTT_MODEL) + .query("select sum(sal)\n" + + "from scott.emp \n" + + "group by ename limit 10") + .planHasSql("SELECT SUM(\"SAL\")\n" + + "FROM \"SCOTT\".\"EMP\"\n" + + "GROUP BY \"ENAME\"\n" + + "LIMIT 10"); + } + @Test public void testPushDownSort() { CalciteAssert.model(JdbcTest.SCOTT_MODEL) .query("select ename \n" diff --git a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java index 4098989bf0f4..d8b3cc84c225 100644 --- a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java +++ b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java @@ -468,14 +468,14 @@ private Fluent pig(String script) { + "HIREDATE, SAL, COMM, DEPTNO)) AS A\n" + " FROM scott.EMP\n" + " GROUP BY DEPTNO) AS $cor4,\n" - + " LATERAL (SELECT COLLECT(ROW(ENAME, JOB, DEPTNO, SAL)) AS X\n" - + " FROM (SELECT ENAME, JOB, DEPTNO, SAL\n" + + " LATERAL (SELECT X\n" + + " FROM (SELECT 'all' AS $f0, COLLECT(ROW(ENAME, JOB, DEPTNO, SAL)) AS X\n" + " FROM UNNEST (SELECT $cor4.A AS $f0\n" + " FROM (VALUES (0)) AS t (ZERO)) " + "AS t2 (EMPNO, ENAME, JOB, MGR, HIREDATE, SAL, COMM, DEPTNO)\n" + " WHERE JOB <> 'CLERK'\n" - + " ORDER BY SAL) AS t5\n" - + " GROUP BY 'all') AS t8) AS $cor5,\n" + + " GROUP BY 'all'\n" + + " ORDER BY SAL) AS t7) AS t8) AS $cor5,\n" + " LATERAL UNNEST (SELECT $cor5.X AS $f0\n" + " FROM (VALUES (0)) AS t (ZERO)) " + "AS t11 (ENAME, JOB, DEPTNO, SAL) AS t110\n" From e3fddf2d4d4c8e9e6b627bd090e647e19f922397 Mon Sep 17 00:00:00 2001 From: Lei Jiang Date: Mon, 6 Jan 2020 18:57:56 +0800 Subject: [PATCH 2/8] [CALCITE-3662] Generate wrong SQL when plan contains Project(Sort(Aggregate)) and aggregate field has no alias (Lei Jiang) --- .../rel/rel2sql/RelToSqlConverter.java | 6 +++++- .../apache/calcite/test/JdbcAdapterTest.java | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java index 8fe2abb31b64..3eb28c065cd5 100644 --- a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java +++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java @@ -924,7 +924,11 @@ public List createAsFullOperands(RelDataType rowType, SqlNode leftOpera final String lowerName = name.toLowerCase(Locale.ROOT); if (lowerName.startsWith("expr$")) { // Put it in ordinalMap - ordinalMap.put(lowerName, node); + if (stack.size() != 1) { + node = as(node, name); + } else { + ordinalMap.put(lowerName, node); + } } else if (alias == null || !alias.equals(name)) { node = as(node, name); } diff --git a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java index 7d09fca60a84..8567ff76031e 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java @@ -188,6 +188,27 @@ public class JdbcAdapterTest { + "LIMIT 10"); } + @Test public void testSelectImplicitAliasByStar() { + CalciteAssert.model(JdbcTest.FOODMART_MODEL) + .query( + "select \"product_id\" + 1, * from (" + + "select * from\n" + + "(" + + "select \"product_id\", sum(\"store_sales\") from \"sales_fact_1997\" group by \"product_id\") A\n" + + "union all\n" + + "select * from (\n" + + "select \"product_id\", sum(\"store_sales\") from \"sales_fact_1997\" group by \"product_id\") B\n" + + ") C") + .planHasSql("SELECT \"product_id\" + 1, \"product_id\", \"EXPR$1\"\n" + + "FROM (SELECT \"product_id\", COALESCE(SUM(\"store_sales\"), 0) AS \"EXPR$1\"\n" + + "FROM \"foodmart\".\"sales_fact_1997\"\n" + + "GROUP BY \"product_id\"\n" + + "UNION ALL\n" + + "SELECT \"product_id\", COALESCE(SUM(\"store_sales\"), 0) AS \"EXPR$1\"\n" + + "FROM \"foodmart\".\"sales_fact_1997\"\n" + + "GROUP BY \"product_id\") AS \"t1\""); + } + @Test public void testPushDownSort() { CalciteAssert.model(JdbcTest.SCOTT_MODEL) .query("select ename \n" From 48cb514134105953cc45945289cb322828eb29fd Mon Sep 17 00:00:00 2001 From: Lei Jiang Date: Wed, 8 Jan 2020 15:05:07 +0800 Subject: [PATCH 3/8] [CALCITE-3662] Generate wrong SQL when plan contains Project(Sort(Aggregate)) and aggregate field has no alias (Lei Jiang) --- .../rel/rel2sql/RelToSqlConverter.java | 18 +++++---------- .../rel/rel2sql/RelToSqlConverterTest.java | 23 +++++++++++++++---- .../apache/calcite/test/JdbcAdapterTest.java | 21 ----------------- 3 files changed, 25 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java index 3eb28c065cd5..a6adb2a192d1 100644 --- a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java +++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java @@ -230,7 +230,6 @@ public Result visit(Project e) { final Builder builder; if (e.getInput() instanceof Sort) { builder = x.builder(e); - builder.clauses.add(Clause.SELECT); } else { builder = x.builder(e, Clause.SELECT); } @@ -639,15 +638,6 @@ public Result visit(Sort e) { } Result x = visitChild(0, e.getInput()); Builder builder = x.builder(e, Clause.ORDER_BY); - if (stack.size() != 1 && builder.select.getSelectList() == null) { - // Generates explicit column names instead of start(*) for - // non-root order by to avoid ambiguity. - final List selectList = Expressions.list(); - for (RelDataTypeField field : e.getRowType().getFieldList()) { - addSelect(selectList, builder.context.field(field.getIndex()), e.getRowType()); - } - builder.select.setSelectList(new SqlNodeList(selectList, POS)); - } List orderByList = Expressions.list(); for (RelFieldCollation field : e.getCollation().getFieldCollations()) { builder.addOrderItem(orderByList, field); @@ -923,8 +913,12 @@ public List createAsFullOperands(RelDataType rowType, SqlNode leftOpera String alias = SqlValidatorUtil.getAlias(node, -1); final String lowerName = name.toLowerCase(Locale.ROOT); if (lowerName.startsWith("expr$")) { - // Put it in ordinalMap - if (stack.size() != 1) { + final ArrayDeque clone = new ArrayDeque<>(stack); + clone.pop(); + final Frame parent = clone.peek(); + if (parent != null && parent.r instanceof Union) { + // For case: Project(Union(expression without alias)) + // Project should use the alias to fill its select-list. node = as(node, name); } else { ordinalMap.put(lowerName, node); diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index 13785d0f4f55..e39d4792dfd6 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -1145,10 +1145,10 @@ private static String toSql(RelNode root, SqlDialect dialect) { @Test public void testUnionOperatorForBigQuery() { final String query = "select mod(11,3) from \"product\"\n" + "UNION select 1 from \"product\""; - final String expected = "SELECT MOD(11, 3)\n" + final String expected = "SELECT MOD(11, 3) AS `EXPR$0`\n" + "FROM foodmart.product\n" + "UNION DISTINCT\n" - + "SELECT 1\n" + + "SELECT 1 AS `EXPR$0`\n" + "FROM foodmart.product"; sql(query).withBigQuery().ok(expected); } @@ -1156,10 +1156,10 @@ private static String toSql(RelNode root, SqlDialect dialect) { @Test public void testUnionAllOperatorForBigQuery() { final String query = "select mod(11,3) from \"product\"\n" + "UNION ALL select 1 from \"product\""; - final String expected = "SELECT MOD(11, 3)\n" + final String expected = "SELECT MOD(11, 3) AS `EXPR$0`\n" + "FROM foodmart.product\n" + "UNION ALL\n" - + "SELECT 1\n" + + "SELECT 1 AS `EXPR$0`\n" + "FROM foodmart.product"; sql(query).withBigQuery().ok(expected); } @@ -4344,6 +4344,21 @@ private void checkLiteral2(String expression, String expected) { sql(query).ok(expected); } + @Test public void testSelectImplicitAliasFromUnionByStar() { + final String query = "select \"shelf_width\", * from (\n" + + "select \"shelf_width\", \"product_id\" + 1 from \"product\"\n" + + "union all\n" + + "select \"shelf_width\", \"product_id\" from \"product\"" + + ") A"; + final String expected = "SELECT \"shelf_width\", \"shelf_width\" AS \"shelf_width0\", \"EXPR$1\"\n" + + "FROM (SELECT \"shelf_width\", \"product_id\" + 1 AS \"EXPR$1\"\n" + + "FROM \"foodmart\".\"product\"\n" + + "UNION ALL\n" + + "SELECT \"shelf_width\", \"product_id\"\n" + + "FROM \"foodmart\".\"product\") AS \"t1\""; + sql(query).ok(expected); + } + /** Fluid interface to run tests. */ static class Sql { private final SchemaPlus schema; diff --git a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java index 8567ff76031e..7d09fca60a84 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java @@ -188,27 +188,6 @@ public class JdbcAdapterTest { + "LIMIT 10"); } - @Test public void testSelectImplicitAliasByStar() { - CalciteAssert.model(JdbcTest.FOODMART_MODEL) - .query( - "select \"product_id\" + 1, * from (" - + "select * from\n" - + "(" - + "select \"product_id\", sum(\"store_sales\") from \"sales_fact_1997\" group by \"product_id\") A\n" - + "union all\n" - + "select * from (\n" - + "select \"product_id\", sum(\"store_sales\") from \"sales_fact_1997\" group by \"product_id\") B\n" - + ") C") - .planHasSql("SELECT \"product_id\" + 1, \"product_id\", \"EXPR$1\"\n" + - "FROM (SELECT \"product_id\", COALESCE(SUM(\"store_sales\"), 0) AS \"EXPR$1\"\n" + - "FROM \"foodmart\".\"sales_fact_1997\"\n" + - "GROUP BY \"product_id\"\n" + - "UNION ALL\n" + - "SELECT \"product_id\", COALESCE(SUM(\"store_sales\"), 0) AS \"EXPR$1\"\n" + - "FROM \"foodmart\".\"sales_fact_1997\"\n" + - "GROUP BY \"product_id\") AS \"t1\""); - } - @Test public void testPushDownSort() { CalciteAssert.model(JdbcTest.SCOTT_MODEL) .query("select ename \n" From 4c05dd9183b4423054cdf7bd66f57877550b302b Mon Sep 17 00:00:00 2001 From: Lei Jiang Date: Thu, 9 Jan 2020 16:14:08 +0800 Subject: [PATCH 4/8] [CALCITE-3662] Generate wrong SQL when plan contains Project(Sort(Aggregate)) and aggregate field has no alias (Lei Jiang) --- .../rel/rel2sql/RelToSqlConverter.java | 8 ++---- .../rel/rel2sql/RelToSqlConverterTest.java | 26 +++++++++---------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java index a6adb2a192d1..ced34e63e88d 100644 --- a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java +++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java @@ -913,12 +913,8 @@ public List createAsFullOperands(RelDataType rowType, SqlNode leftOpera String alias = SqlValidatorUtil.getAlias(node, -1); final String lowerName = name.toLowerCase(Locale.ROOT); if (lowerName.startsWith("expr$")) { - final ArrayDeque clone = new ArrayDeque<>(stack); - clone.pop(); - final Frame parent = clone.peek(); - if (parent != null && parent.r instanceof Union) { - // For case: Project(Union(expression without alias)) - // Project should use the alias to fill its select-list. + final RelNode current = stack.peek().r; + if (stack.size() != 1 && current instanceof Aggregate) { node = as(node, name); } else { ordinalMap.put(lowerName, node); diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index e39d4792dfd6..c4305b36f134 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -1145,10 +1145,10 @@ private static String toSql(RelNode root, SqlDialect dialect) { @Test public void testUnionOperatorForBigQuery() { final String query = "select mod(11,3) from \"product\"\n" + "UNION select 1 from \"product\""; - final String expected = "SELECT MOD(11, 3) AS `EXPR$0`\n" + final String expected = "SELECT MOD(11, 3)\n" + "FROM foodmart.product\n" + "UNION DISTINCT\n" - + "SELECT 1 AS `EXPR$0`\n" + + "SELECT 1\n" + "FROM foodmart.product"; sql(query).withBigQuery().ok(expected); } @@ -1156,10 +1156,10 @@ private static String toSql(RelNode root, SqlDialect dialect) { @Test public void testUnionAllOperatorForBigQuery() { final String query = "select mod(11,3) from \"product\"\n" + "UNION ALL select 1 from \"product\""; - final String expected = "SELECT MOD(11, 3) AS `EXPR$0`\n" + final String expected = "SELECT MOD(11, 3)\n" + "FROM foodmart.product\n" + "UNION ALL\n" - + "SELECT 1 AS `EXPR$0`\n" + + "SELECT 1\n" + "FROM foodmart.product"; sql(query).withBigQuery().ok(expected); } @@ -2488,7 +2488,7 @@ private void checkLiteral2(String expression, String expected) { + "where b.\"product_id\" = a.\"product_id\")"; String expected = "SELECT \"product_name\"\n" + "FROM \"foodmart\".\"product\"\n" - + "WHERE EXISTS (SELECT COUNT(*)\n" + + "WHERE EXISTS (SELECT COUNT(*) AS \"EXPR$0\"\n" + "FROM \"foodmart\".\"sales_fact_1997\"\n" + "WHERE \"product_id\" = \"product\".\"product_id\")"; sql(query).config(NO_EXPAND_CONFIG).ok(expected); @@ -2501,7 +2501,7 @@ private void checkLiteral2(String expression, String expected) { + "where b.\"product_id\" = a.\"product_id\")"; String expected = "SELECT \"product_name\"\n" + "FROM \"foodmart\".\"product\"\n" - + "WHERE NOT EXISTS (SELECT COUNT(*)\n" + + "WHERE NOT EXISTS (SELECT COUNT(*) AS \"EXPR$0\"\n" + "FROM \"foodmart\".\"sales_fact_1997\"\n" + "WHERE \"product_id\" = \"product\".\"product_id\")"; sql(query).config(NO_EXPAND_CONFIG).ok(expected); @@ -3751,7 +3751,7 @@ private void checkLiteral2(String expression, String expected) { + " from \"department\") as t(did)"; final String expected = "SELECT \"col_0\" + 1\n" - + "FROM UNNEST (SELECT COLLECT(\"department_id\")\n" + + "FROM UNNEST (SELECT COLLECT(\"department_id\") AS \"EXPR$0\"\n" + "FROM \"foodmart\".\"department\") AS \"t0\" (\"col_0\")"; sql(sql).ok(expected); } @@ -4029,11 +4029,11 @@ private void checkLiteral2(String expression, String expected) { + " where A.\"department_id\" = ( select min( A.\"department_id\") from \"foodmart\".\"department\" B where 1=2 )"; final String expected = "SELECT \"employee\".\"department_id\"\n" + "FROM \"foodmart\".\"employee\"\n" - + "INNER JOIN (SELECT \"t1\".\"department_id\" \"department_id0\", MIN(\"t1\".\"department_id\")\n" + + "INNER JOIN (SELECT \"t1\".\"department_id\" \"department_id0\", MIN(\"t1\".\"department_id\") \"EXPR$0\"\n" + "FROM (SELECT NULL \"department_id\", NULL \"department_description\"\nFROM \"DUAL\"\nWHERE 1 = 0) \"t\",\n" + "(SELECT \"department_id\"\nFROM \"foodmart\".\"employee\"\nGROUP BY \"department_id\") \"t1\"\n" + "GROUP BY \"t1\".\"department_id\") \"t3\" ON \"employee\".\"department_id\" = \"t3\".\"department_id0\"" - + " AND \"employee\".\"department_id\" = MIN(\"t1\".\"department_id\")"; + + " AND \"employee\".\"department_id\" = \"t3\".\"EXPR$0\""; sql(query).withOracle().ok(expected); } @@ -4044,7 +4044,7 @@ private void checkLiteral2(String expression, String expected) { final String expected = "SELECT \"employee\".\"department_id\"\n" + "FROM \"foodmart\".\"employee\"\n" + "INNER JOIN (SELECT \"t1\".\"department_id\" AS \"department_id0\"," - + " MIN(\"t1\".\"department_id\")\n" + + " MIN(\"t1\".\"department_id\") AS \"EXPR$0\"\n" + "FROM (SELECT *\nFROM (VALUES (NULL, NULL))" + " AS \"t\" (\"department_id\", \"department_description\")" + "\nWHERE 1 = 0) AS \"t\"," @@ -4052,7 +4052,7 @@ private void checkLiteral2(String expression, String expected) { + "\nGROUP BY \"department_id\") AS \"t1\"" + "\nGROUP BY \"t1\".\"department_id\") AS \"t3\" " + "ON \"employee\".\"department_id\" = \"t3\".\"department_id0\"" - + " AND \"employee\".\"department_id\" = MIN(\"t1\".\"department_id\")"; + + " AND \"employee\".\"department_id\" = \"t3\".\"EXPR$0\""; sql(query).ok(expected); } @@ -4350,8 +4350,8 @@ private void checkLiteral2(String expression, String expected) { + "union all\n" + "select \"shelf_width\", \"product_id\" from \"product\"" + ") A"; - final String expected = "SELECT \"shelf_width\", \"shelf_width\" AS \"shelf_width0\", \"EXPR$1\"\n" - + "FROM (SELECT \"shelf_width\", \"product_id\" + 1 AS \"EXPR$1\"\n" + final String expected = "SELECT \"shelf_width\", \"shelf_width\" AS \"shelf_width0\", \"product_id\" + 1\n" + + "FROM (SELECT \"shelf_width\", \"product_id\" + 1\n" + "FROM \"foodmart\".\"product\"\n" + "UNION ALL\n" + "SELECT \"shelf_width\", \"product_id\"\n" From e6943c91e74fdd445e757528e9cf8c021e195224 Mon Sep 17 00:00:00 2001 From: Lei Jiang Date: Thu, 9 Jan 2020 17:01:33 +0800 Subject: [PATCH 5/8] [CALCITE-3662] Generate wrong SQL when plan contains Project(Sort(Aggregate)) and aggregate field has no alias (Lei Jiang) --- .../calcite/rel/rel2sql/RelToSqlConverter.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java index ced34e63e88d..a53a0551a96b 100644 --- a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java +++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java @@ -227,12 +227,8 @@ public Result visit(Project e) { if (isStar(e.getChildExps(), e.getInput().getRowType(), e.getRowType())) { return x; } - final Builder builder; - if (e.getInput() instanceof Sort) { - builder = x.builder(e); - } else { - builder = x.builder(e, Clause.SELECT); - } + final Builder builder = + x.builder(e, Clause.SELECT); final List selectList = new ArrayList<>(); for (RexNode ref : e.getChildExps()) { SqlNode sqlExpr = builder.context.toSql(null, ref); @@ -638,6 +634,15 @@ public Result visit(Sort e) { } Result x = visitChild(0, e.getInput()); Builder builder = x.builder(e, Clause.ORDER_BY); + if (stack.size() != 1 && builder.select.getSelectList() == null) { + // Generates explicit column names instead of start(*) for + // non-root order by to avoid ambiguity. + final List selectList = Expressions.list(); + for (RelDataTypeField field : e.getRowType().getFieldList()) { + addSelect(selectList, builder.context.field(field.getIndex()), e.getRowType()); + } + builder.select.setSelectList(new SqlNodeList(selectList, POS)); + } List orderByList = Expressions.list(); for (RelFieldCollation field : e.getCollation().getFieldCollations()) { builder.addOrderItem(orderByList, field); From ba08a0cbee19f87f12844b4f053c384396fa4673 Mon Sep 17 00:00:00 2001 From: Lei Jiang Date: Thu, 9 Jan 2020 17:35:00 +0800 Subject: [PATCH 6/8] [CALCITE-3662] Generate wrong SQL when plan contains Project(Sort(Aggregate)) and aggregate field has no alias (Lei Jiang) --- .../java/org/apache/calcite/test/JdbcAdapterTest.java | 5 +++-- .../test/java/org/apache/calcite/test/PigRelOpTest.java | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java index 7d09fca60a84..7b73ae072228 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java @@ -182,10 +182,11 @@ public class JdbcAdapterTest { .query("select sum(sal)\n" + "from scott.emp \n" + "group by ename limit 10") - .planHasSql("SELECT SUM(\"SAL\")\n" + .planHasSql("SELECT \"EXPR$0\"\n" + + "FROM (SELECT \"ENAME\", SUM(\"SAL\") AS \"EXPR$0\"\n" + "FROM \"SCOTT\".\"EMP\"\n" + "GROUP BY \"ENAME\"\n" - + "LIMIT 10"); + + "LIMIT 10) AS \"t0\""); } @Test public void testPushDownSort() { diff --git a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java index d8b3cc84c225..4098989bf0f4 100644 --- a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java +++ b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java @@ -468,14 +468,14 @@ private Fluent pig(String script) { + "HIREDATE, SAL, COMM, DEPTNO)) AS A\n" + " FROM scott.EMP\n" + " GROUP BY DEPTNO) AS $cor4,\n" - + " LATERAL (SELECT X\n" - + " FROM (SELECT 'all' AS $f0, COLLECT(ROW(ENAME, JOB, DEPTNO, SAL)) AS X\n" + + " LATERAL (SELECT COLLECT(ROW(ENAME, JOB, DEPTNO, SAL)) AS X\n" + + " FROM (SELECT ENAME, JOB, DEPTNO, SAL\n" + " FROM UNNEST (SELECT $cor4.A AS $f0\n" + " FROM (VALUES (0)) AS t (ZERO)) " + "AS t2 (EMPNO, ENAME, JOB, MGR, HIREDATE, SAL, COMM, DEPTNO)\n" + " WHERE JOB <> 'CLERK'\n" - + " GROUP BY 'all'\n" - + " ORDER BY SAL) AS t7) AS t8) AS $cor5,\n" + + " ORDER BY SAL) AS t5\n" + + " GROUP BY 'all') AS t8) AS $cor5,\n" + " LATERAL UNNEST (SELECT $cor5.X AS $f0\n" + " FROM (VALUES (0)) AS t (ZERO)) " + "AS t11 (ENAME, JOB, DEPTNO, SAL) AS t110\n" From c6c1e72ed3fa7442ed7a58620227d7c72cea1430 Mon Sep 17 00:00:00 2001 From: Lei Jiang Date: Thu, 9 Jan 2020 17:49:15 +0800 Subject: [PATCH 7/8] [CALCITE-3662] Generate wrong SQL when plan contains Project(Sort(Aggregate)) and aggregate field has no alias (Lei Jiang) --- .../rel/rel2sql/RelToSqlConverterTest.java | 19 +++++++++++++++++++ .../apache/calcite/test/JdbcAdapterTest.java | 12 ------------ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index c4305b36f134..5600d646cc35 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -21,11 +21,13 @@ import org.apache.calcite.plan.RelOptRule; import org.apache.calcite.plan.RelTraitDef; import org.apache.calcite.plan.hep.HepPlanner; +import org.apache.calcite.plan.hep.HepProgram; import org.apache.calcite.plan.hep.HepProgramBuilder; import org.apache.calcite.rel.RelNode; import org.apache.calcite.rel.core.JoinRelType; import org.apache.calcite.rel.rules.ProjectToWindowRule; import org.apache.calcite.rel.rules.PruneEmptyRules; +import org.apache.calcite.rel.rules.SortProjectTransposeRule; import org.apache.calcite.rel.rules.UnionMergeRule; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeFactory; @@ -4359,6 +4361,23 @@ private void checkLiteral2(String expression, String expected) { sql(query).ok(expected); } + @Test public void testGroupByWithLimitPlan() { + final HepProgram program = HepProgram + .builder() + .addRuleInstance(SortProjectTransposeRule.INSTANCE) + .build(); + final HepPlanner planner = new HepPlanner(program); + final String query = "SELECT sum(\"salary\") FROM \"employee\" group by \"gender\" limit 10"; + final String expected = "SELECT \"EXPR$0\"\n" + + "FROM (SELECT \"gender\", SUM(\"salary\") AS \"EXPR$0\"\n" + + "FROM \"foodmart\".\"employee\"\n" + + "GROUP BY \"gender\"\n" + + "FETCH NEXT 10 ROWS ONLY) AS \"t1\""; + sql(query) + .optimize(RuleSets.ofList(SortProjectTransposeRule.INSTANCE), planner) + .ok(expected); + } + /** Fluid interface to run tests. */ static class Sql { private final SchemaPlus schema; diff --git a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java index 7b73ae072228..361aa4a301d4 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java @@ -177,18 +177,6 @@ public class JdbcAdapterTest { + "ON \"t\".\"DEPTNO\" = \"t0\".\"DEPTNO\""); } - @Test public void testGroupByWithLimitPlan() { - CalciteAssert.model(JdbcTest.SCOTT_MODEL) - .query("select sum(sal)\n" - + "from scott.emp \n" - + "group by ename limit 10") - .planHasSql("SELECT \"EXPR$0\"\n" - + "FROM (SELECT \"ENAME\", SUM(\"SAL\") AS \"EXPR$0\"\n" - + "FROM \"SCOTT\".\"EMP\"\n" - + "GROUP BY \"ENAME\"\n" - + "LIMIT 10) AS \"t0\""); - } - @Test public void testPushDownSort() { CalciteAssert.model(JdbcTest.SCOTT_MODEL) .query("select ename \n" From bfc724612e35a557c53b95e28ff98ba1c1959ddd Mon Sep 17 00:00:00 2001 From: Lei Jiang Date: Thu, 9 Jan 2020 18:32:51 +0800 Subject: [PATCH 8/8] [CALCITE-3662] Generate wrong SQL when plan contains Project(Sort(Aggregate)) and aggregate field has no alias (Lei Jiang) --- .../org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index 5600d646cc35..807d933d7ba3 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -4352,7 +4352,8 @@ private void checkLiteral2(String expression, String expected) { + "union all\n" + "select \"shelf_width\", \"product_id\" from \"product\"" + ") A"; - final String expected = "SELECT \"shelf_width\", \"shelf_width\" AS \"shelf_width0\", \"product_id\" + 1\n" + final String expected = "SELECT \"shelf_width\", \"shelf_width\" AS \"shelf_width0\"" + + ", \"product_id\" + 1\n" + "FROM (SELECT \"shelf_width\", \"product_id\" + 1\n" + "FROM \"foodmart\".\"product\"\n" + "UNION ALL\n"