[
https://issues.apache.org/jira/browse/CALCITE-5296?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17611718#comment-17611718
]
Julian Hyde commented on CALCITE-5296:
--------------------------------------
[~wojustme], My mistake. The fix is good, the test cases are good. I withdraw
my -1.
I'm sorry I used strong language. That is unacceptable behavior, and I will try
to do better.
I think [~libenchao] has legitimate concerns that {{scope}} should be changed
to {{selectScope}} elsewhere in the method. The process I would follow is:
change {{scope}} to {{selectScope}} in one place, run the test suite, and if
nothing breaks, try to devise a test where that line of code would make a
difference (in other words, to prove that the new code is better).
Since the bug is in the validator, the right place for the test is in the
validator unit test, not in SqlToRelConverterTest. I would also add an
end-to-end SQL test:
{noformat}
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index b981d12dc5..bee41dc4fd 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -7293,6 +7293,13 @@ public boolean isBangEqualAllowed() {
+ "group by rollup(empno), deptno")
.ok()
.type("RecordType(INTEGER NOT NULL DEPTNO, INTEGER EMPNO) NOT NULL");
+
+ // DEPTNO becomes NULL because it is rolled up, and so does DEPTNO + 1.
+ sql("select deptno, deptno + 1 AS d1\n"
+ + "from emp\n"
+ + "group by rollup(deptno)")
+ .ok()
+ .type("RecordType(INTEGER DEPTNO, INTEGER D1) NOT NULL");
}
@Test void testGroupByCorrelatedColumn() {
diff --git a/core/src/test/resources/sql/agg.iq
b/core/src/test/resources/sql/agg.iq
index ec9b01987e..867071d6f2 100644
--- a/core/src/test/resources/sql/agg.iq
+++ b/core/src/test/resources/sql/agg.iq
@@ -566,6 +566,26 @@ having count(*) > 3;
!ok
+# ROLLUP column used in expression; see
+# [CALCITE-5296] In a query with ROLLUP, validator wrongly infers that a
column is NOT NULL
+select deptno, deptno + 1 as d1
+from emp
+group by rollup(deptno);
++--------+----+
+| DEPTNO | D1 |
++--------+----+
+| 10 | 11 |
+| 20 | 21 |
+| 30 | 31 |
+| 50 | 51 |
+| 60 | 61 |
+| | |
+| | |
++--------+----+
+(7 rows)
+
+!ok
+
# CUBE and DISTINCT
select distinct count(*) from emp group by cube(deptno, gender);
+--------+ {noformat}
> In a query with ROLLUP, validator wrongly infers that a column is NOT NULL
> --------------------------------------------------------------------------
>
> Key: CALCITE-5296
> URL: https://issues.apache.org/jira/browse/CALCITE-5296
> Project: Calcite
> Issue Type: Bug
> Components: core
> Reporter: Xurenhe
> Assignee: Xurenhe
> Priority: Major
> Labels: pull-request-available
> Attachments: image-2022-09-29-16-31-04-642.png, screenshot-1.png
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> CALCITE throws exception during the stage of sql_to_rel, when executing
> *SELECT* statement after the statement of {*}ROLLUP{*}, and grouping's item
> exists in the *SELECT* statement.
>
> {*}Error message{*}:
> {code:java}
> Conversion to relational algebra failed to preserve datatypes:
> validated type:
> RecordType(INTEGER DEPTNO, INTEGER NOT NULL EXPR$1) NOT NULL
> converted type:
> RecordType(INTEGER DEPTNO, INTEGER EXPR$1) NOT NULL
> rel:
> LogicalProject(DEPTNO=[$0], EXPR$1=[CASE(=($2, 0), $0, 1)])
> LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}, {}]],
> agg#0=[GROUPING($0)])
> LogicalProject(DEPTNO=[$7], JOB=[$2])
> LogicalTableScan(table=[[CATALOG, SALES,
> EMP]])java.lang.AssertionError: Conversion to relational algebra failed to
> preserve datatypes:
> validated type:
> RecordType(INTEGER DEPTNO, INTEGER NOT NULL EXPR$1) NOT NULL
> converted type:
> RecordType(INTEGER DEPTNO, INTEGER EXPR$1) NOT NULL
> rel:
> LogicalProject(DEPTNO=[$0], EXPR$1=[CASE(=($2, 0), $0, 1)])
> LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}, {}]],
> agg#0=[GROUPING($0)])
> LogicalProject(DEPTNO=[$7], JOB=[$2])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]]) at
> org.apache.calcite.sql2rel.SqlToRelConverter.checkConvertedType(SqlToRelConverter.java:492)
> at
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:607)
> at
> org.apache.calcite.sql.test.AbstractSqlTester.convertSqlToRel2(AbstractSqlTester.java:536)
> at
> org.apache.calcite.sql.test.AbstractSqlTester.assertSqlConvertsTo(AbstractSqlTester.java:477)
> at
> org.apache.calcite.sql.test.AbstractSqlTester.assertConvertsTo(AbstractSqlTester.java:455)
> at
> org.apache.calcite.test.SqlToRelFixture.convertsTo(SqlToRelFixture.java:106)
> at org.apache.calcite.test.SqlToRelFixture.ok(SqlToRelFixture.java:94)
> at
> org.apache.calcite.test.SqlToRelConverterTest.testCaseWhenGroupingSet(SqlToRelConverterTest.java:4687)
> {code}
>
> {*}Test
> case{*}:(org.apache.calcite.test.SqlToRelConverterTest#testCaseWhenGroupingSet)
> {code:java}
> // JAVA: org.apache.calcite.test.SqlToRelConverterTest#testCaseWhenGroupingSet
> @Test void testCaseWhenGroupingSet() {
> final String sql = "select deptno, case when grouping(deptno) = 0 then
> deptno else 1 end\n"
> + "from emp\n"
> + "group by rollup(deptno, job)";
> sql(sql).ok();
> }
> // XML: org/apache/calcite/test/SqlToRelConverterTest.xml:456
> <TestCase name="testCaseWhenGroupingSet">
> <Resource name="sql">
> <![CDATA[SELECT deptno, CASE WHEN grouping(deptno) = 0 THEN deptno ELSE
> 1 END
> FROM emp
> GROUP BY ROLLUP(deptno, job)]]>
> </Resource>
> <Resource name="plan">
> <![CDATA[
> LogicalProject(DEPTNO=[$0], EXPR$1=[CASE(=($2, 0), $0, 1)])
> LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}, {}]],
> agg#0=[GROUPING($0)])
> LogicalProject(DEPTNO=[$7], JOB=[$2])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> ]]>
> </Resource>
> </TestCase>{code}
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
