kosiew commented on code in PR #24357:
URL: https://github.com/apache/datafusion/pull/24357#discussion_r3801709616
##########
datafusion/sqllogictest/test_files/window.slt:
##########
@@ -6874,3 +6874,138 @@ ORDER BY id
2 NULL 3 3 3 3
3 NULL NULL NULL NULL NULL
4 7 7 7 7 7
+
+# RANGE window frame over a binary ORDER BY key. The default frame for an
+# ORDER BY without an explicit frame is RANGE BETWEEN UNBOUNDED PRECEDING AND
+# CURRENT ROW, which used to fail with
+# "Internal error: Cannot run range queries on datatype: Binary".
+# Binary is orderable, so peer/range comparison is well defined just like Utf8.
+query ?I
+SELECT x, COUNT(*) OVER (ORDER BY x)
+FROM (VALUES (arrow_cast('a', 'Binary')),
+ (arrow_cast('b', 'Binary')),
+ (arrow_cast('b', 'Binary')),
+ (arrow_cast('c', 'Binary'))) t(x)
+ORDER BY x
+----
+61 1
+62 3
+62 3
+63 4
+
+query ?I
+SELECT x, COUNT(*) OVER (ORDER BY x DESC RANGE BETWEEN UNBOUNDED PRECEDING AND
CURRENT ROW)
+FROM (VALUES (arrow_cast('a', 'LargeBinary')),
+ (arrow_cast('b', 'LargeBinary')),
+ (arrow_cast('b', 'LargeBinary'))) t(x)
+ORDER BY x
+----
+61 3
+62 2
+62 2
+
+query ?I
+SELECT x, COUNT(*) OVER (ORDER BY x RANGE BETWEEN CURRENT ROW AND UNBOUNDED
FOLLOWING)
+FROM (VALUES (arrow_cast('a', 'BinaryView')),
+ (arrow_cast('b', 'BinaryView')),
+ (arrow_cast('b', 'BinaryView'))) t(x)
+ORDER BY x
+----
+61 3
+62 2
+62 2
+
+query ?I
+SELECT x, COUNT(*) OVER (ORDER BY x)
+FROM (VALUES (arrow_cast(arrow_cast('a', 'Binary'), 'FixedSizeBinary(1)')),
+ (arrow_cast(arrow_cast('b', 'Binary'), 'FixedSizeBinary(1)')))
t(x)
+ORDER BY x
+----
+61 1
+62 2
+
+# A binary ORDER BY key nested in a dictionary resolves through the same arm.
+query ?I
+SELECT x, COUNT(*) OVER (ORDER BY x)
+FROM (VALUES (arrow_cast(arrow_cast('a', 'Binary'), 'Dictionary(Int32,
Binary)')),
+ (arrow_cast(arrow_cast('b', 'Binary'), 'Dictionary(Int32,
Binary)')),
+ (arrow_cast(arrow_cast('b', 'Binary'), 'Dictionary(Int32,
Binary)'))) t(x)
+ORDER BY x
+----
+61 1
+62 3
+62 3
+
+# A non-aggregate window function over a binary ORDER BY key.
+query ?I
+SELECT x, RANK() OVER (ORDER BY x)
+FROM (VALUES (arrow_cast('a', 'Binary')),
+ (arrow_cast('b', 'Binary')),
+ (arrow_cast('b', 'Binary'))) t(x)
+ORDER BY x
+----
+61 1
+62 2
+62 2
+
+# Unsupported RANGE ORDER BY types still propagate the type-coercion error.
+query error DataFusion error: type_coercion\ncaused by\nInternal error: Cannot
run range queries on datatype: Struct\("c0": Int64\)\.
+SELECT COUNT(*) OVER (ORDER BY x)
+FROM (VALUES (struct(1))) t(x)
+
+# An order key only needs to be comparable for a free range frame, whose bounds
+# are all UNBOUNDED or CURRENT ROW. A finite offset such as `1 PRECEDING`
instead
+# has to be computed as `current_value - 1`, so it additionally requires
+# arithmetic on the order key type. Reject such frames during planning for
every
+# order key type that has no arithmetic, rather than silently widening the
frame
+# to the whole partition.
+query error DataFusion error: type_coercion\ncaused by\nError during planning:
RANGE with offset PRECEDING/FOLLOWING is not supported for ORDER BY type Binary
Review Comment:
Could we add a small negative case for `Dictionary(Int32, Binary)` with
`RANGE BETWEEN 1 PRECEDING AND CURRENT ROW`? The positive dictionary case
already checks that the type is unwrapped correctly for a free frame. This
would also cover the combination of dictionary unwrapping and the new
finite-offset arithmetic check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
