Skip to content

Commit

Permalink
Merge pull request #10181 from hawkfish/window-first
Browse files Browse the repository at this point in the history
Internal #1022: Window TIME RANGE
  • Loading branch information
Mytherin committed Jan 15, 2024
2 parents a056c8e + 3a353bb commit e165741
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 3 deletions.
24 changes: 21 additions & 3 deletions 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"
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down
37 changes: 37 additions & 0 deletions test/sql/window/test_window_range.test
Expand Up @@ -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

Expand Down

0 comments on commit e165741

Please sign in to comment.