zabetak commented on a change in pull request #2274:
URL: https://github.com/apache/calcite/pull/2274#discussion_r527569313



##########
File path: 
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
##########
@@ -3008,6 +3008,12 @@ private void checkLiteral2(String expression, String 
expected) {
             + "OVER (PARTITION BY \"hire_date\" ORDER BY \"employee_id\"), 
\"hire_date\"\n"
             + "FROM \"foodmart\".\"employee\"\n"
             + "GROUP BY \"hire_date\", \"employee_id\"";
+    String query7 = "SELECT "
+        + "count(distinct \"employee_id\") over (order by \"hire_date\") FROM 
\"employee\"";
+    String expected7 = "SELECT "
+        + "COUNT(DISTINCT \"employee_id\") "
+        + "OVER (ORDER BY \"hire_date\" RANGE BETWEEN UNBOUNDED PRECEDING AND 
CURRENT ROW) AS \"$0\""
+        + "\nFROM \"foodmart\".\"employee\"";

Review comment:
       This is not something that changed by this PR so I don't think its a 
problem.

##########
File path: core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
##########
@@ -1007,19 +1007,27 @@ private SqlCall toSql(RexProgram program, RexOver 
rexOver) {
           orderList, isRows, lowerBound, upperBound, allowPartial, POS);
 
       final List<SqlNode> nodeList = toSql(program, rexOver.getOperands());
-      return createOverCall(sqlAggregateFunction, nodeList, sqlWindow);
+      return createOverCall(sqlAggregateFunction, nodeList, sqlWindow, 
rexOver.isDistinct());
     }
 
     private SqlCall createOverCall(SqlAggFunction op, List<SqlNode> operands,
-        SqlWindow window) {
+        SqlWindow window, boolean isDistinct) {
       if (op instanceof SqlSumEmptyIsZeroAggFunction) {
         // Rewrite "SUM0(x) OVER w" to "COALESCE(SUM(x) OVER w, 0)"
         final SqlCall node =
-            createOverCall(SqlStdOperatorTable.SUM, operands, window);
+            createOverCall(SqlStdOperatorTable.SUM, operands, window, 
isDistinct);

Review comment:
       Can we enter here with `isDistinct == true`? If yes can you also add a 
testcase?




----------------------------------------------------------------
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.

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


Reply via email to