asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1314536702
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -2716,11 +2716,25 @@ private void
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
* correctly ignores an {@code EXCEPT ALL}. It can only handle
* {@code EXCEPT DISTINCT}. */
@Test void testMinusToDistinctAll() {
- final String sql = "select EMPNO,ENAME,JOB from emp where deptno = 10\n"
+ final String sql = "select EMPNO, ENAME, JOB from emp where deptno = 10\n"
+ "except\n"
- + "select EMPNO,ENAME,JOB from emp where deptno = 20\n"
+ + "select EMPNO, ENAME, JOB from emp where deptno = 20\n"
+ "except all\n"
- + "select EMPNO,ENAME,JOB from emp where deptno = 30\n";
+ + "select EMPNO, ENAME, JOB from emp where deptno = 30\n";
+ sql(sql)
+ .withRule(CoreRules.MINUS_MERGE, CoreRules.MINUS_TO_DISTINCT,
CoreRules.PROJECT_MERGE)
+ .check();
+ }
+
+ /** Tests {@link org.apache.calcite.rel.rules.MinusToDistinctRule},
+ * correctly ignores an {@code EXCEPT ALL}. It can only handle
+ * {@code EXCEPT DISTINCT}. */
+ @Test void testMinusToDistinctAll1() {
Review Comment:
Nit: same comment about test names
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -3803,6 +3803,42 @@ public void checkOrderBy(final boolean desc,
"empid=110; name=Theodore");
}
+ @Test void testMinusToDistinct1() {
Review Comment:
Nit: different tests are covering for different aspects of the feature, if
possible it's better to convey what's the specificity of the test in its name,
instead of appending a counter.
In this case, I'd say the focus is involving a subquery, so what about
`testMinusToDistinctWithSubquery` or similar?
##########
core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml:
##########
@@ -6639,6 +6639,49 @@ LogicalMinus(all=[true])
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
LogicalFilter(condition=[=($7, 30)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testMinusToDistinctAll1">
+ <Resource name="sql">
+ <![CDATA[select EMPNO, ENAME, JOB from emp where deptno = 10
+except all
+select EMPNO, ENAME, JOB from emp where deptno = 20
+except
+select EMPNO, ENAME, JOB from emp where deptno = 30
+]]>
+ </Resource>
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalMinus(all=[false])
+ LogicalMinus(all=[true])
+ LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ LogicalFilter(condition=[=($7, 10)])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ LogicalFilter(condition=[=($7, 20)])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ LogicalFilter(condition=[=($7, 30)])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
Review Comment:
Shouldn't we still have a `LogicalMinus` here associated to the `minus all`
in the original plan?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
