viirya opened a new issue, #25856: URL: https://github.com/apache/datafusion/issues/25856
### Describe the bug `date_bin` accepts negative strides, but their semantics are not defined anywhere: they are not documented or tested, and the only check is that the stride is non-zero. PostgreSQL rejects strides that are not positive. The current results are inconsistent and, for month strides, not monotonic: - **Negative fixed-duration strides** round toward the origin (`compute_distance`), so for a source before the origin the bin is after the source. - **Negative month strides**: in `bin_months`, the "move back one bin" step subtracts the stride, so a negative stride moves the bin forward instead. The output is not monotonic even for ordinary dates. ### To Reproduce ```sql -- Fixed stride: the bin is after the source select date_bin(interval '-1 hour', timestamp '2023-01-01 00:00:00', timestamp '2023-01-01 00:30:00'); -- 2023-01-01T00:30:00 (with '1 hour': 2022-12-31T23:30:00) -- Month stride: ascending input, output out of order select date_bin(interval '-1 month', column1, timestamp '2023-01-31 00:00:00') from (values (timestamp '2023-01-01 00:00:00'), (timestamp '2023-01-31 00:00:00')); -- 2023-02-28T00:00:00 -- 2023-01-31T00:00:00 -- (with '1 month': 2022-12-31T00:00:00, 2023-01-31T00:00:00) ``` ### Expected behavior This needs a decision. The options I see: 1. **Reject non-positive strides**, as PostgreSQL does. This turns queries that currently run into errors. 2. **Use the absolute value of the stride.** The bins `origin + k * stride` are the same set for `stride` and `-stride`, so every bin is the latest one at or before the source. This is monotonic and never after the source, which matches rules 1 and 4 of @mhilton's proposal in https://github.com/apache/datafusion/issues/10602#issuecomment-5845897142. It changes the results of negative fixed strides as well as month strides. 3. **Make negative month strides round toward the origin**, like negative fixed strides. This is monotonic, but keeps bins after the source. Options 1 and 2 give `date_bin` a result that is never after its source; 2 also keeps existing queries working, and was suggested by @mhilton in https://github.com/apache/datafusion/pull/25815#discussion_r4123839765. ### Additional context #25815 stops `date_bin` from propagating the ordering of its source for negative month strides, so the non-monotonic output no longer produces misordered query results. Once negative strides are monotonic, that special case in `output_ordering` can be removed. With option 2, `compute_distance` and `compute_distance_wide` could also use `rem_euclid`. -- 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]
