From 9d7e9589656ef1626306c4375ad56e81a43b5a57 Mon Sep 17 00:00:00 2001 From: Richard Wesley Date: Wed, 27 Mar 2024 10:54:13 -0700 Subject: [PATCH] Issue #10885: Negative Window RANGEs Detect and trap negative range boundaries to match PG behaviour. fixes: #10885 fixes: duckdblabs/duckdb-internal#1389 --- src/execution/window_executor.cpp | 76 +++++++++++++++--------- test/sql/window/test_negative_range.test | 63 ++++++++++++++++++++ 2 files changed, 111 insertions(+), 28 deletions(-) create mode 100644 test/sql/window/test_negative_range.test diff --git a/src/execution/window_executor.cpp b/src/execution/window_executor.cpp index 7fe581095742..627e8df630da 100644 --- a/src/execution/window_executor.cpp +++ b/src/execution/window_executor.cpp @@ -199,17 +199,34 @@ struct OperationCompare : public std::function { template static idx_t FindTypedRangeBound(const WindowInputColumn &over, const idx_t order_begin, const idx_t order_end, - WindowInputExpression &boundary, const idx_t chunk_idx, const FrameBounds &prev) { + const WindowBoundary range, WindowInputExpression &boundary, const idx_t chunk_idx, + const FrameBounds &prev) { D_ASSERT(!boundary.CellIsNull(chunk_idx)); const auto val = boundary.GetCell(chunk_idx); OperationCompare comp; - WindowColumnIterator begin(over, order_begin); - WindowColumnIterator end(over, order_end); + + // Check that the value we are searching for is in range. + if (range == WindowBoundary::EXPR_PRECEDING_RANGE) { + // Preceding but value past the end + const auto cur_val = over.GetCell(order_end); + if (comp(cur_val, val)) { + throw OutOfRangeException("Invalid RANGE PRECEDING value"); + } + } else { + // Following but value before beginning + D_ASSERT(range == WindowBoundary::EXPR_FOLLOWING_RANGE); + const auto cur_val = over.GetCell(order_begin); + if (comp(val, cur_val)) { + throw OutOfRangeException("Invalid RANGE FOLLOWING value"); + } + } // Try to reuse the previous bounds to restrict the search. // This is only valid if the previous bounds were non-empty // Only inject the comparisons if the previous bounds are a strict subset. + WindowColumnIterator begin(over, order_begin); + WindowColumnIterator end(over, order_end); if (prev.start < prev.end) { if (order_begin < prev.start && prev.start < order_end) { const auto first = over.GetCell(prev.start); @@ -237,37 +254,40 @@ static idx_t FindTypedRangeBound(const WindowInputColumn &over, const idx_t orde template static idx_t FindRangeBound(const WindowInputColumn &over, const idx_t order_begin, const idx_t order_end, - WindowInputExpression &boundary, const idx_t chunk_idx, const FrameBounds &prev) { + const WindowBoundary range, WindowInputExpression &boundary, const idx_t chunk_idx, + const FrameBounds &prev) { D_ASSERT(boundary.chunk.ColumnCount() == 1); D_ASSERT(boundary.chunk.data[0].GetType().InternalType() == over.input_expr.ptype); switch (over.input_expr.ptype) { case PhysicalType::INT8: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case PhysicalType::INT16: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case PhysicalType::INT32: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case PhysicalType::INT64: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case PhysicalType::UINT8: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case PhysicalType::UINT16: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case PhysicalType::UINT32: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case PhysicalType::UINT64: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case PhysicalType::INT128: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case PhysicalType::UINT128: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, + prev); case PhysicalType::FLOAT: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case PhysicalType::DOUBLE: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case PhysicalType::INTERVAL: - return FindTypedRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindTypedRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, + prev); default: throw InternalException("Unsupported column type for RANGE"); } @@ -275,13 +295,13 @@ static idx_t FindRangeBound(const WindowInputColumn &over, const idx_t order_beg template static idx_t FindOrderedRangeBound(const WindowInputColumn &over, const OrderType range_sense, const idx_t order_begin, - const idx_t order_end, WindowInputExpression &boundary, const idx_t chunk_idx, - const FrameBounds &prev) { + const idx_t order_end, const WindowBoundary range, WindowInputExpression &boundary, + const idx_t chunk_idx, const FrameBounds &prev) { switch (range_sense) { case OrderType::ASCENDING: - return FindRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); case OrderType::DESCENDING: - return FindRangeBound(over, order_begin, order_end, boundary, chunk_idx, prev); + return FindRangeBound(over, order_begin, order_end, range, boundary, chunk_idx, prev); default: throw InternalException("Unsupported ORDER BY sense for RANGE"); } @@ -458,7 +478,7 @@ void WindowBoundariesState::Update(const idx_t row_idx, const WindowInputColumn window_start = peer_start; } else { prev.start = FindOrderedRangeBound(range_collection, range_sense, valid_start, row_idx, - boundary_start, chunk_idx, prev); + start_boundary, boundary_start, chunk_idx, prev); window_start = prev.start; } break; @@ -467,8 +487,8 @@ void WindowBoundariesState::Update(const idx_t row_idx, const WindowInputColumn if (boundary_start.CellIsNull(chunk_idx)) { window_start = peer_start; } else { - prev.start = FindOrderedRangeBound(range_collection, range_sense, row_idx, valid_end, boundary_start, - chunk_idx, prev); + prev.start = FindOrderedRangeBound(range_collection, range_sense, row_idx, valid_end, start_boundary, + boundary_start, chunk_idx, prev); window_start = prev.start; } break; @@ -502,8 +522,8 @@ void WindowBoundariesState::Update(const idx_t row_idx, const WindowInputColumn if (boundary_end.CellIsNull(chunk_idx)) { window_end = peer_end; } else { - prev.end = FindOrderedRangeBound(range_collection, range_sense, valid_start, row_idx, boundary_end, - chunk_idx, prev); + prev.end = FindOrderedRangeBound(range_collection, range_sense, valid_start, row_idx, end_boundary, + boundary_end, chunk_idx, prev); window_end = prev.end; } break; @@ -512,8 +532,8 @@ void WindowBoundariesState::Update(const idx_t row_idx, const WindowInputColumn if (boundary_end.CellIsNull(chunk_idx)) { window_end = peer_end; } else { - prev.end = FindOrderedRangeBound(range_collection, range_sense, row_idx, valid_end, boundary_end, - chunk_idx, prev); + prev.end = FindOrderedRangeBound(range_collection, range_sense, row_idx, valid_end, end_boundary, + boundary_end, chunk_idx, prev); window_end = prev.end; } break; diff --git a/test/sql/window/test_negative_range.test b/test/sql/window/test_negative_range.test new file mode 100644 index 000000000000..a7dc98ff3ace --- /dev/null +++ b/test/sql/window/test_negative_range.test @@ -0,0 +1,63 @@ +# name: test/sql/window/test_negative_range.test +# description: Check that negative ranges trigger errors +# group: [window] + +statement ok +PRAGMA enable_verification + +statement ok +CREATE OR REPLACE TABLE issue10855(i INTEGER, v FLOAT); + +statement ok +INSERT INTO issue10855 VALUES (0, 1), (1, 2), (2, 3),; + +# Ascending +statement error +SELECT i, v, sum(v) OVER (ORDER BY i RANGE BETWEEN 1 PRECEDING AND -1 FOLLOWING) +FROM issue10855 +---- +Out of Range Error: Invalid RANGE FOLLOWING value + +statement error +SELECT i, v, sum(v) OVER (ORDER BY i RANGE BETWEEN -1 FOLLOWING AND 1 FOLLOWING) +FROM issue10855 +---- +Out of Range Error: Invalid RANGE FOLLOWING value + +statement error +SELECT i, v, sum(v) OVER (ORDER BY i RANGE BETWEEN -1 PRECEDING AND 1 FOLLOWING) +FROM issue10855 +---- +Out of Range Error: Invalid RANGE PRECEDING value + +statement error +SELECT i, v, sum(v) OVER (ORDER BY i RANGE BETWEEN 1 PRECEDING AND -1 PRECEDING) +FROM issue10855 +---- +Out of Range Error: Invalid RANGE PRECEDING value + +# Descending +statement error +SELECT i, v, sum(v) OVER (ORDER BY i DESC RANGE BETWEEN 1 PRECEDING AND -1 FOLLOWING) +FROM issue10855 +---- +Out of Range Error: Invalid RANGE FOLLOWING value + +statement error +SELECT i, v, sum(v) OVER (ORDER BY i DESC RANGE BETWEEN -1 FOLLOWING AND 1 FOLLOWING) +FROM issue10855 +---- +Out of Range Error: Invalid RANGE FOLLOWING value + +statement error +SELECT i, v, sum(v) OVER (ORDER BY i DESC RANGE BETWEEN -1 PRECEDING AND 1 FOLLOWING) +FROM issue10855 +---- +Out of Range Error: Invalid RANGE PRECEDING value + +statement error +SELECT i, v, sum(v) OVER (ORDER BY i DESC RANGE BETWEEN 1 PRECEDING AND -1 PRECEDING) +FROM issue10855 +---- +Out of Range Error: Invalid RANGE PRECEDING value +