mihaibudiu commented on code in PR #3735:
URL: https://github.com/apache/calcite/pull/3735#discussion_r1529232018


##########
core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml:
##########
@@ -2856,6 +2856,32 @@ LogicalAggregate(group=[{}], EXPR$0=[MIN($1) FILTER $3], 
EXPR$1=[COUNT($0) FILTE
     LogicalAggregate(group=[{1, 2}], groups=[[{1, 2}, {}]], EXPR$0=[SUM($0)], 
$g=[GROUPING($1, $2)])
       LogicalProject(COMM=[$6], SAL=[$5], $f2=[>($5, 1000)])
         LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testDistinctWithFilterWithoutGroupByUsingJoin">
+    <Resource name="sql">
+      <![CDATA[SELECT SUM(comm), COUNT(DISTINCT sal) FILTER (WHERE sal > 1000)
+FROM emp]]>
+    </Resource>
+    <Resource name="planBefore">
+      <![CDATA[
+LogicalAggregate(group=[{}], EXPR$0=[SUM($0)], EXPR$1=[COUNT(DISTINCT $1) 
FILTER $2])
+  LogicalProject(COMM=[$6], SAL=[$5], $f2=[>($5, 1000)])
+    LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+    </Resource>
+    <Resource name="planAfter">
+      <![CDATA[
+LogicalJoin(condition=[true], joinType=[inner])

Review Comment:
   My mistake.
   
   Should we strive to address [CALCITE-6332] in this PR as well? I saw you 
commented on it.
   As far as I can tell, a rule can check some preconditions and refuse to be 
applied if they are not satisfied. That would be one way to solve 6332.
   
   As for this particular fix, don't know what happens if you have multiple 
aggregates each with a different condition, as the other above comment from 
@kgyrtkirk points out.
   
   One solution would be to refuse to apply this rule when such combinations of 
filters exist.
   
   I also sent a question to the dev list about the ability to test such rules 
using Quidem tests. That would be great, but I don't know if it's possible yet.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to