Make negative range frame throw user check failed error not memory le…

…ak exception in ubuntu 20 os
facebookincubator · Aug 27, 2024 · eb32cdb · eb32cdb
1 parent 3bf1a74
commit eb32cdb
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 2 deletions.
diff --git a/velox/exec/Window.cpp b/velox/exec/Window.cpp
@@ -153,12 +153,14 @@ Window::WindowFrame Window::createWindowFrame(
     }
   };
 
+  auto startFrameArg = createFrameChannelArg(frame.startValue);
+  auto endFrameArg = createFrameChannelArg(frame.endValue);
   return WindowFrame(
       {frame.type,
        frame.startType,
        frame.endType,
-       createFrameChannelArg(frame.startValue),
-       createFrameChannelArg(frame.endValue)});
+       std::move(startFrameArg),
+       std::move(endFrameArg)});
 }
 
 void Window::createWindowFunctions() {

diff --git a/velox/exec/tests/WindowTest.cpp b/velox/exec/tests/WindowTest.cpp
@@ -439,5 +439,68 @@ TEST_F(WindowTest, duplicateOrOverlappingKeys) {
       "Sorting keys must be unique and not overlap with partitioning keys. Found duplicate key: b");
 }
 
+TEST_F(WindowTest, nagativeFrameArg) {
+  const vector_size_t size = 1'000;
+
+  auto sizeAt = [](vector_size_t row) { return row % 5; };
+  auto keyAt = [](vector_size_t row) { return row % 11; };
+  auto valueAt = [](vector_size_t row) { return row % 13; };
+  auto keys = makeArrayVector<float>(size, sizeAt, keyAt);
+  auto data = makeRowVector(
+      {"c0", "c1", "p0", "p1", "k0", "row_number"},
+      {
+          // Payload.
+          makeFlatVector<float>(size, [](auto row) { return row; }),
+          makeFlatVector<float>(size, [](auto row) { return row; }),
+          // Partition key.
+          keys,
+          makeFlatVector<std::string>(
+              size, [](auto row) { return fmt::format("{}", row + 20); }),
+          makeFlatVector<int32_t>(size, [](auto row) { return row; }),
+          // Sorting key.
+          makeFlatVector<int64_t>(size, [](auto row) { return row; }),
+      });
+
+  createDuckDbTable({data});
+
+  // Define a range of negative values to test for the window frame start and
+  // end.
+  std::vector<int64_t> negativeOffsets = {-1, 2, -3};
+
+  for (int64_t startOffset : negativeOffsets) {
+    for (int64_t endOffset : negativeOffsets) {
+      auto plan =
+          PlanBuilder()
+              .values(split(data, 10))
+              .window({fmt::format(
+                  "regr_count(c0, c1) over (partition by p0, p1 order by row_number ROWS between {} PRECEDING and {} FOLLOWING)",
+                  startOffset,
+                  endOffset)})
+              .planNode();
+
+      if (startOffset < 0) {
+        VELOX_ASSERT_THROW(
+            AssertQueryBuilder(plan, duckDbQueryRunner_)
+                .assertResults(fmt::format(
+                    "SELECT *, regr_count(c0, c1) over (partition by p0, p1 order by row_number  ROWS between {} PRECEDING and {} FOLLOWING) from tmp",
+                    startOffset,
+                    endOffset)),
+            fmt::format(
+                "Window frame {} offset must not be negative", startOffset));
+
+      } else if (endOffset < 0) {
+        VELOX_ASSERT_THROW(
+            AssertQueryBuilder(plan, duckDbQueryRunner_)
+                .assertResults(fmt::format(
+                    "SELECT *, regr_count(c0, c1) over (partition by p0, p1 order by row_number  ROWS between {} PRECEDING and {} FOLLOWING) from tmp",
+                    startOffset,
+                    endOffset)),
+            fmt::format(
+                "Window frame {} offset must not be negative", endOffset));
+      }
+    }
+  }
+}
+
 } // namespace
 } // namespace facebook::velox::exec