alamb commented on code in PR #26070:
URL: https://github.com/apache/datafusion/pull/26070#discussion_r4242780706
##########
datafusion/functions/src/datetime/date_bin.rs:
##########
@@ -419,19 +396,10 @@ fn date_bin_nanos_interval_wide(
Some(origin + time_delta)
}
-// `compute_distance` in i128. `stride` is non-zero, and `time_diff` is far
+// `compute_distance` in i128. `stride` is positive, and `time_diff` is far
Review Comment:
this is much nicer
##########
docs/source/library-user-guide/upgrading/56.0.0.md:
##########
@@ -611,6 +611,37 @@ should treat an empty list as the absent-key case instead.
See [issue #24981](https://github.com/apache/datafusion/issues/24981) and
[issue #24983](https://github.com/apache/datafusion/issues/24983) for details.
+### `date_bin` rejects strides that are not positive
+
+`date_bin` now returns an error when its stride is not greater than zero.
+This matches PostgreSQL for fixed strides (PostgreSQL does not support month
+strides at all), and DataFusion applies the same rule to month strides. A zero
+stride was already an error; its message is now
Review Comment:
I am not sure we need to explicitly say the error message
##########
docs/source/library-user-guide/upgrading/56.0.0.md:
##########
@@ -611,6 +611,37 @@ should treat an empty list as the absent-key case instead.
See [issue #24981](https://github.com/apache/datafusion/issues/24981) and
[issue #24983](https://github.com/apache/datafusion/issues/24983) for details.
+### `date_bin` rejects strides that are not positive
+
+`date_bin` now returns an error when its stride is not greater than zero.
+This matches PostgreSQL for fixed strides (PostgreSQL does not support month
Review Comment:
Perhaps we can link to the posgres docs for this claim:
https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-BIN
##########
docs/source/library-user-guide/upgrading/56.0.0.md:
##########
@@ -611,6 +611,37 @@ should treat an empty list as the absent-key case instead.
See [issue #24981](https://github.com/apache/datafusion/issues/24981) and
[issue #24983](https://github.com/apache/datafusion/issues/24983) for details.
+### `date_bin` rejects strides that are not positive
+
+`date_bin` now returns an error when its stride is not greater than zero.
+This matches PostgreSQL for fixed strides (PostgreSQL does not support month
+strides at all), and DataFusion applies the same rule to month strides. A zero
+stride was already an error; its message is now
+`DATE_BIN stride must be greater than zero`.
+
+Previously, a negative stride was accepted but its bins were not well defined.
+It returned the same bin as the positive stride only when the timestamp was at
Review Comment:
rather than explain in detail what used to happen, I think we could just
link to the ticket and cut this entry down a buncg
--
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]
