Skip to content

Commit bac3c01

Browse files
viiryaviirya
committed
test: simplify date_bin overflow comments and tests
Address review feedback: shorten the output_ordering comment, drop the history of old results from the overflow test comments, use a displayable out-of-range value instead of casting i64::MAX seconds to Int64, and explain why a negative month stride is not monotonic.
1 parent 8ef201a commit bac3c01

3 files changed

Lines changed: 17 additions & 24 deletions

File tree

‎datafusion/functions/src/datetime/date_bin.rs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -273,16 +273,11 @@ impl ScalarUDFImpl for DateBinFunc {
273273
let date_value = &input[1];
274274
let reference = input.get(2);
275275

276-
// DATE_BIN preserves the order of its second argument. Values whose
277-
// nanosecond form overflows i64 are binned in i128, so a non-null
278-
// input only becomes NULL when its bin cannot be represented: a bin
279-
// starting before the minimum value of the type, or a month bin
280-
// outside the range of `DateTime<Utc>`. These extremes are accepted
281-
// rather than giving up the ordering for all inputs.
276+
// DATE_BIN preserves the order of its second argument.
282277
//
283278
// A negative month stride can move a bin past its source (see
284279
// `bin_months`), so its output is not monotonic, even for ordinary
285-
// dates. Only propagate the ordering for strides known not to be one.
280+
// dates.
286281
let monotonic_stride =
287282
matches!(step.range.lower(), ScalarValue::IntervalDayTime(Some(_)))
288283
|| matches!(

‎datafusion/sqllogictest/test_files/date_bin_errors.slt‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,7 @@ select date_bin(
7373
----
7474
NULL
7575

76-
# Extreme timestamp overflow: source - origin overflows i64, so the bin is
77-
# computed in i128 instead (previously NULL, and before that a panic)
76+
# source - origin overflows i64 nanoseconds
7877
query P
7978
select date_bin(
8079
interval '1 nanosecond',
@@ -94,27 +93,25 @@ select date_bin(
9493
----
9594
NULL
9695

97-
# Source timestamp scaling to nanoseconds overflows, so the bin is computed in
98-
# i128 instead (previously NULL, and before that a panic). The result cannot be
99-
# displayed as a date, so show its raw value.
100-
query I
101-
select arrow_cast(date_bin(
96+
# Scaling the source to nanoseconds overflows i64
97+
query P
98+
select date_bin(
10299
interval '1 nanosecond',
103-
arrow_cast(9223372036854775807, 'Timestamp(Second, None)'),
100+
arrow_cast(10000000000, 'Timestamp(Second, None)'),
104101
timestamp '1970-01-01 00:00:00'
105-
), 'Int64');
102+
);
106103
----
107-
9223372036854775807
104+
2286-11-20T17:46:40
108105

109106
# Same in the array path
110-
query I
111-
select arrow_cast(date_bin(
107+
query P
108+
select date_bin(
112109
interval '1 nanosecond',
113110
ts,
114111
timestamp '1970-01-01 00:00:00'
115-
), 'Int64')
112+
)
116113
from (
117-
values (arrow_cast(9223372036854775807, 'Timestamp(Second, None)'))
114+
values (arrow_cast(10000000000, 'Timestamp(Second, None)'))
118115
) as t(ts);
119116
----
120-
9223372036854775807
117+
2286-11-20T17:46:40

‎datafusion/sqllogictest/test_files/datetime/timestamps.slt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1821,8 +1821,9 @@ ORDER BY b ASC NULLS LAST
18211821
1970-01-01T00:00:00
18221822
2286-11-20T17:46:40
18231823

1824-
# A negative month stride can move a bin past its source, so date_bin keeps
1825-
# the final sort for it.
1824+
# A negative month stride does not shift every bin by the same amount: here
1825+
# 2023-01-01 bins forward to 2023-02-28, while 2023-01-31 bins to itself. The
1826+
# output is not sorted, so date_bin keeps the final sort for it.
18261827
query TT
18271828
EXPLAIN SELECT date_bin(INTERVAL '-1 month', ts, TIMESTAMP '2023-01-31 00:00:00') AS b
18281829
FROM (

0 commit comments

Comments
 (0)