Fix the Window operator with overflowed empty k-rows frames (facebook…

…incubator#9476)

Summary:
Pull Request resolved: facebookincubator#9476

The boundaries of Window frames are int32 integers. When the frame
boundaries given in the query exceed int32 limit, the Window
operator needs to adjust the frame bounds. However, the current
code has a bug that when the frame end is below INT32_MIN, it
adjust the frame end to 0. This is wrong because the original frame is
empty, but after the adjustment, it always include row 0. This diff fixes
this bug by setting the frame bound to -1 when the frame bound
belows INT32_MIN. A subsequent call to computeValidFrames will
check whether this frame is empty and mark it properly.

This diff fixes facebookincubator#9375.

Reviewed By: Yuhta

Differential Revision: D56085211

fbshipit-source-id: 7bc330a331e12e1997b71130ab5c8267a8e75e01

velox/exec/Window.cpp

@@ -282,7 +282,9 @@ void updateKRowsOffsetsColumn(
   for (auto i = 0; i < numRows; i++) {
     auto startValue = (int64_t)(startRow + i) + precedingFactor * offsets[i];
     if (startValue < INT32_MIN) {
-      rawFrameBounds[i] = 0;
+      // Same as the handling of startValue < INT32_MIN in
+      // updateKRowsFrameBounds.
+      rawFrameBounds[i] = -1;
     } else if (startValue > INT32_MAX) {
       // computeValidFrames will replace INT32_MAX set here
       // with partition's final row index.
@@ -308,10 +310,15 @@ void Window::updateKRowsFrameBounds(
 
     if (isKPreceding) {
       if (startValue < INT32_MIN) {
-        // For overflow in kPreceding frames, k < INT32_MIN. Since the max
-        // number of rows in a partition is INT32_MAX, the frameBound will
-        // always be bound to the first row of the partition
-        std::fill_n(rawFrameBounds, numRows, 0);
+        // For overflow in kPreceding frames, i.e., k < INT32_MIN, we set the
+        // frame bound to -1. For frames whose original frame start is below
+        // INT32_MIN, the new frame start becomes -1 and will be corrected to 0
+        // by the subsequent computeValidFrames call. For frames whose original
+        // frame end is below INT32_MIN, the new frame end becomes -1 and will
+        // be marked invalid by the subsequent computeValidFrames call. This is
+        // expected because the max number of rows in a partition is INT32_MAX,
+        // so a frame end below INT32_MIN always results in an empty frame.
+        std::fill_n(rawFrameBounds, numRows, -1);
         return;
       }
       std::iota(rawFrameBounds, rawFrameBounds + numRows, startValue);

velox/functions/prestosql/window/tests/AggregateWindowTest.cpp

@@ -349,6 +349,43 @@ TEST_F(AggregateWindowTest, integerOverflowRowsFrame) {
        makeFlatVector<int64_t>({6, 6, 6, 6, 6, 6, 4, 4, 4, 4})});
   WindowTestBase::testWindowFunction(
       {input}, "count(c1)", overClause, frameClause, expected);
+
+  // Empty frame with overflowed frame end.
+  input = makeRowVector({
+      makeFlatVector<int64_t>({1, 2, 3, 4}),
+      makeConstant<int64_t>(30000000000, 4),
+  });
+  expected = makeRowVector({
+      makeFlatVector<int64_t>({1, 2, 3, 4}),
+      makeConstant<int64_t>(30000000000, 4),
+      makeNullConstant(TypeKind::BIGINT, 4),
+  });
+  WindowTestBase::testWindowFunction(
+      {input},
+      "sum(c0)",
+      "",
+      "rows between unbounded preceding and 30000000000 preceding",
+      expected);
+  WindowTestBase::testWindowFunction(
+      {input},
+      "sum(c0)",
+      "",
+      "rows between unbounded preceding and c1 preceding",
+      expected);
+
+  // Empty frame with overflowed frame start.
+  WindowTestBase::testWindowFunction(
+      {input},
+      "sum(c0)",
+      "",
+      "rows between 30000000000 following and unbounded following",
+      expected);
+  WindowTestBase::testWindowFunction(
+      {input},
+      "sum(c0)",
+      "",
+      "rows between c1 following and unbounded following",
+      expected);
 }
 
 }; // namespace