Refine the code and add tests

PHILO-HE · Mar 13, 2024 · a9130f8 · a9130f8
1 parent ed2a7d2
commit a9130f8
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 16 deletions.
diff --git a/velox/exec/Window.cpp b/velox/exec/Window.cpp
@@ -320,24 +320,26 @@ void Window::updateKRowsFrameBounds(
       // Considers a very large int64 constantOffset is used.
       if (startValue < std::numeric_limits<int32_t>::min()) {
         std::fill_n(rawFrameBounds, numRows, startRow);
-      } else {
-        // Integer overflow cannot happen.
-        std::iota(rawFrameBounds, rawFrameBounds + numRows, startValue);
-      }
-    } else {
-      auto overflowStart = getOverflowStart(constantOffset);
-      if (overflowStart >= 0 && overflowStart < numRows) {
-        std::iota(rawFrameBounds, rawFrameBounds + overflowStart, startValue);
-        // For remaining, set with the largest index for this partition.
-        std::fill_n(
-            rawFrameBounds + overflowStart,
-            numRows - overflowStart,
-            startRow + numRows - 1);
-      } else {
-        // Integer overflow cannot happen.
-        std::iota(rawFrameBounds, rawFrameBounds + numRows, startValue);
+        return;
       }
+      // Integer overflow cannot happen.
+      std::iota(rawFrameBounds, rawFrameBounds + numRows, startValue);
+      return;
+    }
+    // KFollowing.
+    auto overflowStart = getOverflowStart(constantOffset);
+    if (overflowStart >= 0 && overflowStart < numRows) {
+      std::iota(rawFrameBounds, rawFrameBounds + overflowStart, startValue);
+      // For remaining, set with the largest index for this partition.
+      std::fill_n(
+          rawFrameBounds + overflowStart,
+          numRows - overflowStart,
+          startRow + numRows - 1);
+      return;
     }
+    // Integer overflow cannot happen.
+    std::iota(rawFrameBounds, rawFrameBounds + numRows, startValue);
+    return;
   } else {
     currentPartition_->extractColumn(
         frameArg.index, partitionOffset_, numRows, 0, frameArg.value);

diff --git a/velox/functions/prestosql/window/tests/AggregateWindowTest.cpp b/velox/functions/prestosql/window/tests/AggregateWindowTest.cpp
@@ -239,6 +239,18 @@ TEST_F(AggregateWindowTest, integerOverflowRowsFrame) {
   WindowTestBase::testWindowFunction(
       {input}, "count(c1)", overClause, frameClause, expected);
 
+  // Test overflow case that happens during the calculation for the middle
+  // partition.
+  frameClause = "rows between 0 preceding and 2147483645 following";
+  expected = makeRowVector(
+      {c0,
+       c1,
+       c2,
+       c3,
+       makeFlatVector<int64_t>({6, 5, 4, 3, 2, 1, 4, 3, 2, 1})});
+  WindowTestBase::testWindowFunction(
+      {input}, "count(c1)", overClause, frameClause, expected);
+
   // Test integer overflow with column-specified following (int32).
   frameClause = "rows between 0 preceding and c2 following";
   expected = makeRowVector(
@@ -261,6 +273,40 @@ TEST_F(AggregateWindowTest, integerOverflowRowsFrame) {
   WindowTestBase::testWindowFunction(
       {input}, "count(c1)", overClause, frameClause, expected);
 
+  // Test integer overflow with constant preceding (larger than INT32_MAX).
+  frameClause = "rows between 2147483650 preceding and 0 following";
+  expected = makeRowVector(
+      {c0,
+       c1,
+       c2,
+       c3,
+       makeFlatVector<int64_t>({1, 2, 3, 4, 5, 6, 1, 2, 3, 4})});
+  WindowTestBase::testWindowFunction(
+      {input}, "count(c1)", overClause, frameClause, expected);
+
+  // Test overflow case that happens during the calculation for the middle
+  // partition.
+  frameClause = "rows between 2147483645 preceding and 0 following";
+  expected = makeRowVector(
+      {c0,
+       c1,
+       c2,
+       c3,
+       makeFlatVector<int64_t>({1, 2, 3, 4, 5, 6, 1, 2, 3, 4})});
+  WindowTestBase::testWindowFunction(
+      {input}, "count(c1)", overClause, frameClause, expected);
+
+  // Test integer overflow with column-specified preceding (int32).
+  frameClause = "rows between c2 preceding and 0 following";
+  expected = makeRowVector(
+      {c0,
+       c1,
+       c2,
+       c3,
+       makeFlatVector<int64_t>({1, 2, 3, 4, 2, 6, 1, 2, 3, 4})});
+  WindowTestBase::testWindowFunction(
+      {input}, "count(c1)", overClause, frameClause, expected);
+
   // Test integer overflow with column-specified preceding (int64).
   frameClause = "rows between c3 preceding and 0 following";
   expected = makeRowVector(