[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6522: fix: error instead of panic when date_bin interval is 0

["alamb (via GitHub)" <gi...@apache.org>]

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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org