alamb commented on code in PR #6522:
URL: https://github.com/apache/arrow-datafusion/pull/6522#discussion_r1214601551
##########
datafusion/physical-expr/src/datetime_expressions.rs:
##########
@@ -522,6 +522,13 @@ fn date_bin_impl(
let (stride, stride_fn) = stride.bin_fn();
+ // Return error if stride is 0
+ if stride == 0 {
Review Comment:
The error says the stride must be positive and non zero, but this check is
only for `0`
It seems like the main branch supports negative intervals:
```
❯ SELECT DATE_BIN(INTERVAL '-15 minutes', TIMESTAMP '2022-08-03 14:38:50Z',
TIMESTAMP '1970-01-01T00:00:00Z');
+-----------------------------------------------------------------------------------------------------------------+
| datebin(IntervalMonthDayNano("18446743173709551616"),Utf8("2022-08-03
14:38:50Z"),Utf8("1970-01-01T00:00:00Z")) |
+-----------------------------------------------------------------------------------------------------------------+
| 2022-08-03T14:30:00
|
+-----------------------------------------------------------------------------------------------------------------+
1 row in set. Query took 0.001 seconds.
```
Thus, given what @comphead found in
https://github.com/apache/arrow-datafusion/pull/6522#pullrequestreview-1456448347
I think we should either:
1. Update the error to say that the DATE_BIN stride must be non zero
2. Update the code to match the error (prevent negative strides) and then
add test coverage for negative intervals
--
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]
