mhilton commented on code in PR #26153:
URL: https://github.com/apache/datafusion/pull/26153#discussion_r4292979009
##########
datafusion/functions/src/datetime/date_bin.rs:
##########
@@ -463,24 +464,85 @@ fn date_bin_months_interval(stride_months: i64, source:
i64, origin: i64) -> Res
}
}
-// `date_bin_months_interval` with i128 nanoseconds, limited by the range of
-// `DateTime<Utc>` instead of i64 nanoseconds.
+// Compared in field order, which is chronological.
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+struct CivilTimestamp {
+ year: i64,
Review Comment:
Since this has a relatively limited scope, I think it's fine to leave these
types as signed, but a comment saying what are legal and illegal combinations
of signed and unsigned components would be helpful.
##########
datafusion/functions/src/datetime/date_bin.rs:
##########
@@ -1642,10 +1752,10 @@ mod tests {
ScalarValue::new_interval_mdn(0, 0, 3),
ScalarValue::TimestampNanosecond(Some(i64::MIN), None),
),
- // Month bins are limited by the range of `DateTime<Utc>`.
+ // The month bin of Timestamp(Second)::MIN does not fit (#25855).
Review Comment:
I personally think that we should define `date_bin` to use `i64::MIN` for
any value that would be less then `i64::MIN` to ensure that it always
preserves sort order. However that is not really part of this PR.
##########
datafusion/functions/src/datetime/date_trunc.rs:
##########
@@ -644,7 +644,7 @@ const DAYS_PER_ERA: i64 = 146_097;
/// This is a port of Howard Hinnant's `civil_from_days`, which documents the
/// derivation of the constants and the March-based year used below:
/// <https://howardhinnant.github.io/date_algorithms.html#civil_from_days>
-fn civil_from_days(days: i64) -> (i64, i64, i64) {
+pub(super) fn civil_from_days(days: i64) -> (i64, i64, i64) {
Review Comment:
Since this is now being used outside of `date_trunc` maybe it should be
moved to some shared module.
--
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]
