Handle default bounds correctly in WINDOW clause (apache#16833)

When a window is defined as WINDOW W AS <DEF> and using a syntax of (PARTITION BY col1 ORDER BY col2 ROWS x PRECEDING), we would need to default the other bound to CURRENT ROW We already have implemented this earlier, but when defined as WINDOW W AS <DEF>, Calcite takes a different route to validate the window.
sreemanamala · Aug 6, 2024 · ed6b547 · ed6b547
1 parent aeace28
commit ed6b547
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 12 deletions.
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
@@ -133,6 +133,8 @@ public void validateWindow(SqlNode windowOrId, SqlValidatorScope scope, @Nullabl
         throw Util.unexpected(windowOrId.getKind());
     }
 
+    updateBoundsIfNeeded(targetWindow);
+
     @Nullable
     SqlNode lowerBound = targetWindow.getLowerBound();
     @Nullable
@@ -144,17 +146,6 @@ public void validateWindow(SqlNode windowOrId, SqlValidatorScope scope, @Nullabl
       );
     }
 
-    if (lowerBound != null && upperBound == null) {
-      if (lowerBound.getKind() == SqlKind.FOLLOWING || SqlWindow.isUnboundedFollowing(lowerBound)) {
-        upperBound = lowerBound;
-        lowerBound = SqlWindow.createCurrentRow(SqlParserPos.ZERO);
-      } else {
-        upperBound = SqlWindow.createCurrentRow(SqlParserPos.ZERO);
-      }
-      targetWindow.setLowerBound(lowerBound);
-      targetWindow.setUpperBound(upperBound);
-    }
-
     boolean hasBounds = lowerBound != null || upperBound != null;
     if (call.getKind() == SqlKind.NTILE && hasBounds) {
       throw buildCalciteContextException(
@@ -758,6 +749,28 @@ private boolean isUnboundedOrCurrent(@Nullable SqlNode bound)
         || SqlWindow.isUnboundedPreceding(bound);
   }
 
+  /**
+   * Checks if any bound is null and updates with CURRENT ROW
+   */
+  private void updateBoundsIfNeeded(SqlWindow window)
+  {
+    @Nullable
+    SqlNode lowerBound = window.getLowerBound();
+    @Nullable
+    SqlNode upperBound = window.getUpperBound();
+
+    if (lowerBound != null && upperBound == null) {
+      if (lowerBound.getKind() == SqlKind.FOLLOWING || SqlWindow.isUnboundedFollowing(lowerBound)) {
+        upperBound = lowerBound;
+        lowerBound = SqlWindow.createCurrentRow(SqlParserPos.ZERO);
+      } else {
+        upperBound = SqlWindow.createCurrentRow(SqlParserPos.ZERO);
+      }
+      window.setLowerBound(lowerBound);
+      window.setUpperBound(upperBound);
+    }
+  }
+
   @Override
   public void validateCall(SqlCall call, SqlValidatorScope scope)
   {
@@ -812,6 +825,10 @@ protected void validateWindowClause(SqlSelect select)
             sqlNode
         );
       }
+      if (sqlNode instanceof SqlWindow) {
+        SqlWindow window = (SqlWindow) sqlNode;
+        updateBoundsIfNeeded(window);
+      }
     }
     super.validateWindowClause(select);
   }

diff --git a/sql/src/test/resources/calcite/tests/window/defaultBoundCurrentRow.sqlTest b/sql/src/test/resources/calcite/tests/window/defaultBoundCurrentRow.sqlTest
@@ -7,10 +7,11 @@ sql: |
     count(*) OVER (partition by dim2 ORDER BY dim1 ROWS 1 PRECEDING),
     count(*) OVER (partition by dim2 ORDER BY dim1 ROWS CURRENT ROW),
     count(*) OVER (partition by dim2 ORDER BY dim1 ROWS 1 FOLLOWING),
-    count(*) OVER (partition by dim2 ORDER BY dim1 ROWS UNBOUNDED FOLLOWING)
+    count(*) OVER W
   FROM numfoo
   WHERE dim2 IN ('a', 'abc')
   GROUP BY dim2, dim1
+  WINDOW W AS (partition by dim2 ORDER BY dim1 ROWS UNBOUNDED FOLLOWING)
 
 expectedOperators:
   - {"type":"naiveSort","columns":[{"column":"_d1","direction":"ASC"},{"column":"_d0","direction":"ASC"}]}