Skip to content

Commit

Permalink
fix: error instead of panic when date_bin interval is 0 (#6522)
Browse files Browse the repository at this point in the history
* fix: error instead of panic when date_bin interval is 0

* chore: make the error message more accurate

---------

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
  • Loading branch information
NGA-TRAN and alamb committed Jun 2, 2023
1 parent 5ec14e1 commit 859251b
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
21 changes: 19 additions & 2 deletions datafusion/core/tests/sqllogictests/test_files/timestamps.slt
Original file line number Diff line number Diff line change
Expand Up @@ -392,11 +392,28 @@ drop table ts_data_millis
statement ok
drop table ts_data_secs



##########
## test date_bin function
##########

# not support interval 0
statement error Execution error: DATE_BIN stride must be non-zero
SELECT DATE_BIN(INTERVAL '0 second', TIMESTAMP '2022-08-03 14:38:50.000000006Z', TIMESTAMP '1970-01-01T00:00:00Z')

statement error Execution error: DATE_BIN stride must be non-zero
SELECT DATE_BIN(INTERVAL '0 month', TIMESTAMP '2022-08-03 14:38:50.000000006Z')

statement error Execution error: DATE_BIN stride must be non-zero
SELECT
DATE_BIN(INTERVAL '0' minute, time) AS time,
count(val)
FROM (
VALUES
(TIMESTAMP '2021-06-10 17:05:00Z', 0.5),
(TIMESTAMP '2021-06-10 17:19:10Z', 0.3)
) as t (time, val)
group by time;

query P
SELECT DATE_BIN(INTERVAL '15 minutes', TIMESTAMP '2022-08-03 14:38:50Z', TIMESTAMP '1970-01-01T00:00:00Z')
----
Expand Down
18 changes: 18 additions & 0 deletions datafusion/physical-expr/src/datetime_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,13 @@ fn date_bin_impl(

let (stride, stride_fn) = stride.bin_fn();

// Return error if stride is 0
if stride == 0 {
return Err(DataFusionError::Execution(
"DATE_BIN stride must be non-zero".to_string(),
));
}

let f_nanos = |x: Option<i64>| x.map(|x| stride_fn(stride, x, origin));
let f_micros = |x: Option<i64>| {
let scale = 1_000;
Expand Down Expand Up @@ -1029,6 +1036,17 @@ mod tests {
"Execution error: DATE_BIN expects stride argument to be an INTERVAL but got Interval(YearMonth)"
);

// stride: invalid value
let res = date_bin(&[
ColumnarValue::Scalar(ScalarValue::IntervalDayTime(Some(0))),
ColumnarValue::Scalar(ScalarValue::TimestampNanosecond(Some(1), None)),
ColumnarValue::Scalar(ScalarValue::TimestampNanosecond(Some(1), None)),
]);
assert_eq!(
res.err().unwrap().to_string(),
"Execution error: DATE_BIN stride must be non-zero"
);

// stride: overflow of day-time interval
let res = date_bin(&[
ColumnarValue::Scalar(ScalarValue::IntervalDayTime(Some(i64::MAX))),
Expand Down

0 comments on commit 859251b

Please sign in to comment.