This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 379f41d3be465992328d5659ea62b8355e0399d1 Author: Julian Hyde <[email protected]> AuthorDate: Sat May 27 10:50:28 2023 -0700 [CALCITE-5717] RelBuilder.project of literals on a single-row Aggregate should create a Values If RelBuilder.project is applied to an Aggregate with no group keys (which always returns one row) and all of the projected expressions are literals, then the result should be a Values with one row. For example, SELECT 42 FROM myTable GROUP BY () becomes VALUES 42 (Both queries always return one row, even if myTable is empty.) Close apache/calcite#3209 --- .../java/org/apache/calcite/tools/RelBuilder.java | 28 ++++++++++++++++++---- .../calcite/rel/rel2sql/RelToSqlConverterTest.java | 27 ++++++++++++++++++--- .../org/apache/calcite/test/RelBuilderTest.java | 22 +++++++++++++++-- .../apache/calcite/test/SqlToRelConverterTest.java | 4 ++++ .../apache/calcite/test/SqlToRelConverterTest.xml | 5 +--- 5 files changed, 72 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java index f5fa384af4..d745488cb3 100644 --- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java +++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java @@ -2097,17 +2097,19 @@ public class RelBuilder { } // If the expressions are all literals, and the input is a Values with N - // rows, replace with a Values with same tuple N times. + // rows (or an Aggregate with 1 row), replace with a Values with same tuple + // N times. + final int rowCount; if (config.simplifyValues() - && frame.rel instanceof Values - && nodeList.stream().allMatch(e -> e instanceof RexLiteral)) { - final Values values = (Values) build(); + && nodeList.stream().allMatch(e -> e instanceof RexLiteral) + && (rowCount = fixedRowCount(frame)) >= 0) { + RelNode unused = build(); final RelDataTypeFactory.Builder typeBuilder = getTypeFactory().builder(); Pair.forEach(fieldNameList, nodeList, (name, expr) -> typeBuilder.add(requireNonNull(name, "name"), expr.getType())); @SuppressWarnings({"unchecked", "rawtypes"}) final List<RexLiteral> tuple = (List<RexLiteral>) (List) nodeList; - return values(Collections.nCopies(values.tuples.size(), tuple), + return values(Collections.nCopies(rowCount, tuple), typeBuilder.build()); } @@ -2122,6 +2124,22 @@ public class RelBuilder { return this; } + /** If current frame will return a known, constant number of + * rows, returns that number; otherwise returns -1. */ + private static int fixedRowCount(Frame frame) { + if (frame.rel instanceof Values) { + return ((Values) frame.rel).tuples.size(); + } + if (frame.rel instanceof Aggregate) { + final Aggregate aggregate = (Aggregate) frame.rel; + if (aggregate.getGroupSet().isEmpty() + && aggregate.getGroupType() == Aggregate.Group.SIMPLE) { + return 1; + } + } + return -1; + } + /** Creates a {@link Project} of the given * expressions and field names, and optionally optimizing. * 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 3c2d2adc65..53fde5cdf9 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 @@ -694,9 +694,30 @@ class RelToSqlConverterTest { relFn(fn).ok("SELECT \"t\".\"V\" AS \"l_v\"\n" + "FROM (VALUES (1, 2)) AS \"t\" (\"K\", \"V\")\n" + "INNER JOIN " - + "(SELECT 1 AS \"K\"\n" - + "FROM (SELECT COUNT(\"ENAME\") AS \"DUMMY\"\n" - + "FROM \"scott\".\"EMP\") AS \"t0\") AS \"t1\" ON \"t\".\"K\" = \"t1\".\"K\""); + + "(VALUES (1)) AS \"t0\" (\"K\") ON \"t\".\"K\" = \"t0\".\"K\""); + } + + /** As {@link #testTrimmedAggregateUnderProject()} + * but the "COUNT(*) AS DUMMY" field is used. */ + @Test public void testTrimmedAggregateUnderProject2() { + final Function<RelBuilder, RelNode> fn = b -> b + .values(new String[]{"K", "V"}, 1, 2) + .scan("EMP") + .aggregate(b.groupKey(), + b.aggregateCall(SqlStdOperatorTable.COUNT, b.field(1)) + .as("DUMMY")) + .project(b.alias(b.field("DUMMY"), "K")) + .join(JoinRelType.INNER, + b.call(SqlStdOperatorTable.EQUALS, + b.field(2, 0, "K"), + b.field(2, 1, "K"))) + .project(b.alias(b.field(1), "l_v")) + .build(); + // RelFieldTrimmer maybe build the RelNode. + relFn(fn).ok("SELECT \"t\".\"V\" AS \"l_v\"\n" + + "FROM (VALUES (1, 2)) AS \"t\" (\"K\", \"V\")\n" + + "INNER JOIN (SELECT COUNT(\"ENAME\") AS \"DUMMY\"\n" + + "FROM \"scott\".\"EMP\") AS \"t0\" ON \"t\".\"K\" = \"t0\".\"DUMMY\""); } /** Tests GROUP BY ROLLUP of two columns. The SQL for MySQL has diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java index 727fe5ea7b..3000e84d71 100644 --- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java @@ -1356,13 +1356,31 @@ public class RelBuilderTest { RelNode root = builder.scan("EMP") .aggregate(builder.groupKey(), builder.count().as("C")) + .filter( + builder.greaterThan(builder.field("C"), builder.literal(5))) .project(builder.literal(4), builder.literal(2), builder.field(0)) .aggregate(builder.groupKey(builder.field(0), builder.field(1))) .build(); final String expected = "" + "LogicalProject($f0=[4], $f1=[2])\n" - + " LogicalAggregate(group=[{}], C=[COUNT()])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; + + " LogicalFilter(condition=[>($0, 5)])\n" + + " LogicalAggregate(group=[{}], C=[COUNT()])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n"; + assertThat(root, hasTree(expected)); + } + + /** Unlike {@link #testAggregate5()}, where the input to the Aggregate has at + * most one row (zero or one rows), the input is known to be exactly one row, + * and therefore can be converted to {@code Values}. */ + @Test void testAggregate5b() { + final RelBuilder builder = RelBuilder.create(config().build()); + RelNode root = + builder.scan("EMP") + .aggregate(builder.groupKey(), builder.count().as("C")) + .project(builder.literal(4), builder.literal(2), builder.field(0)) + .aggregate(builder.groupKey(builder.field(0), builder.field(1))) + .build(); + final String expected = "LogicalValues(tuples=[[{ 4, 2 }]])\n"; assertThat(root, hasTree(expected)); } diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java index 95b6274a31..4e2c029fb9 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -426,6 +426,10 @@ class SqlToRelConverterTest extends SqlToRelTestBase { sql("select sum(deptno) from emp group by ()").ok(); } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-5717";>[CALCITE-5717] + * RelBuilder.project should generate a Values if all expressions are literals + * and the input is an Aggregate that returns exactly one row</a>. */ @Test void testGroupEmptyYieldLiteral() { // Expected plan is "VALUES 42". The result is one row even if EMP is empty. sql("select 42 from emp group by ()").ok(); diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml index 6902fe5f81..b48f9015ad 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -2350,10 +2350,7 @@ LogicalAggregate(group=[{}], EXPR$0=[SUM($0)]) </Resource> <Resource name="plan"> <![CDATA[ -LogicalProject(EXPR$0=[42]) - LogicalAggregate(group=[{}]) - LogicalProject($f0=[0]) - LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +LogicalValues(tuples=[[{ 42 }]]) ]]> </Resource> </TestCase>
