From 3a353bbe21fd561a83d06c4e4e55a289850722b1 Mon Sep 17 00:00:00 2001
From: Richard Wesley <13156216+hawkfish@users.noreply.github.com>
Date: Tue, 9 Jan 2024 14:58:54 -0800
Subject: [PATCH] Internal #1022: Window TIME RANGE

Time arithmetic is strange and wraps, so we can't use simple arithmetic
to compute the RANGE boundaries. Instead, we bind to epoch + time,
which will not wrap.

fixes: duckdblabs/duckdb-internal#1022
---
 .../expression/bind_window_expression.cpp     | 24 ++++++++++--
 test/sql/window/test_window_range.test        | 37 +++++++++++++++++++
 2 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/src/planner/binder/expression/bind_window_expression.cpp b/src/planner/binder/expression/bind_window_expression.cpp
index 4a66a7fd78d..f8250f0aa83 100644
--- a/src/planner/binder/expression/bind_window_expression.cpp
+++ b/src/planner/binder/expression/bind_window_expression.cpp
@@ -1,4 +1,6 @@
 #include "duckdb/parser/expression/window_expression.hpp"
+#include "duckdb/parser/expression/constant_expression.hpp"
+#include "duckdb/parser/expression/function_expression.hpp"
 #include "duckdb/planner/expression/bound_aggregate_expression.hpp"
 #include "duckdb/planner/expression/bound_cast_expression.hpp"
 #include "duckdb/planner/expression/bound_columnref_expression.hpp"
@@ -122,9 +124,10 @@ BindResult BaseSelectBinder::BindWindow(WindowExpression &window, idx_t depth) {
 		throw BinderException(error_context.FormatError("correlated columns in window functions not supported"));
 	}
 	// If we have range expressions, then only one order by clause is allowed.
-	if ((window.start == WindowBoundary::EXPR_PRECEDING_RANGE || window.start == WindowBoundary::EXPR_FOLLOWING_RANGE ||
-	     window.end == WindowBoundary::EXPR_PRECEDING_RANGE || window.end == WindowBoundary::EXPR_FOLLOWING_RANGE) &&
-	    window.orders.size() != 1) {
+	const auto is_range =
+	    (window.start == WindowBoundary::EXPR_PRECEDING_RANGE || window.start == WindowBoundary::EXPR_FOLLOWING_RANGE ||
+	     window.end == WindowBoundary::EXPR_PRECEDING_RANGE || window.end == WindowBoundary::EXPR_FOLLOWING_RANGE);
+	if (is_range && window.orders.size() != 1) {
 		throw BinderException(error_context.FormatError("RANGE frames must have only one ORDER BY expression"));
 	}
 	// bind inside the children of the window function
@@ -139,6 +142,21 @@ BindResult BaseSelectBinder::BindWindow(WindowExpression &window, idx_t depth) {
 	}
 	for (auto &order : window.orders) {
 		BindChild(order.expression, depth, error);
+
+		//	If the frame is a RANGE frame and the type is a time,
+		//	then we have to convert the time to a timestamp to avoid wrapping.
+		if (!is_range) {
+			continue;
+		}
+		auto &order_expr = order.expression;
+		auto &bound_order = BoundExpression::GetExpression(*order_expr);
+		const auto type_id = bound_order->return_type.id();
+		if (type_id == LogicalTypeId::TIME || type_id == LogicalTypeId::TIME_TZ) {
+			//	Convert to time + epoch and rebind
+			unique_ptr<ParsedExpression> epoch = make_uniq<ConstantExpression>(Value::DATE(date_t::epoch()));
+			BindChild(epoch, depth, error);
+			BindRangeExpression(context, "+", order.expression, epoch);
+		}
 	}
 	BindChild(window.filter_expr, depth, error);
 	BindChild(window.start_expr, depth, error);
diff --git a/test/sql/window/test_window_range.test b/test/sql/window/test_window_range.test
index 590276ea462..9086511392a 100644
--- a/test/sql/window/test_window_range.test
+++ b/test/sql/window/test_window_range.test
@@ -108,6 +108,43 @@ WINDOW win AS ( ORDER BY a
 statement ok
 DROP VIEW c1;
 
+# Time types will wrap unless we bind the ordering as epoch + time
+statement ok
+CREATE TABLE t_time(t TIME);
+
+statement ok
+INSERT INTO t_time VALUES 
+	('12:30:00'),
+    ('22:30:00'),
+    ('13:30:00'),
+    ('01:30:00'),
+    ('15:30:00'),
+    ('20:30:00'),
+    ('04:30:00'),
+    ('06:30:00'),
+    ('18:30:00'),
+    ('21:30:00'),
+    ('00:30:00'),
+    ('00:31:00');
+
+query II
+SELECT t, FIRST_VALUE(t) OVER w AS fv 
+FROM t_time 
+WINDOW w AS (ORDER BY t RANGE INTERVAL 2 HOUR PRECEDING);
+----
+00:30:00	00:30:00
+00:31:00	00:30:00
+01:30:00	00:30:00
+04:30:00	04:30:00
+06:30:00	04:30:00
+12:30:00	12:30:00
+13:30:00	12:30:00
+15:30:00	13:30:00
+18:30:00	18:30:00
+20:30:00	18:30:00
+21:30:00	20:30:00
+22:30:00	20:30:00
+
 # Invalid type coverage
 foreach type boolean varchar