mhilton commented on code in PR #22114:
URL: https://github.com/apache/datafusion/pull/22114#discussion_r4272383558
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -1433,6 +1434,143 @@ impl ScalarValue {
}
}
+ /// Returns a [`ScalarValue`] of type `target_type` representing `date`.
+ ///
+ /// For `Timestamp` types the result is midnight on `date` in the type's
+ /// timezone (or UTC when the type has no timezone).
+ ///
+ /// Returns `None` if `target_type` is not a date or timestamp type, or if
+ /// midnight on `date` cannot be represented such as when the timezone
skips
+ /// midnight that day (for example a daylight saving transition at 00:00).
Review Comment:
How important is this last property (that it returns `None` for date's with
no midnight)? The alternative is to return the first timestamp that does exist
in the day, probably `01:00`. Admittedly that is rather tricky to calculate
just now, but once https://github.com/apache/arrow-rs/pull/11535 is available
it'd be much simpler.
With this current design there is always a risk of a sudden `NULL` appearing
in a sorted array of dates so operators that use this function cannot be known
to preserve the sorting, possibly requiring resorting of sorted data.
##########
datafusion/functions/src/datetime/date_trunc.rs:
##########
@@ -436,6 +452,97 @@ impl ScalarUDFImpl for DateTruncFunc {
&self.aliases
}
+ /// Compute the preimage for `date_trunc(granularity, col) = literal`.
+ ///
+ /// `date_trunc` floors a timestamp to the start of the given period, so
+ /// for any aligned literal `L` the preimage is the half-open interval
+ /// `[L, L + 1 granularity)`.
+ ///
+ /// The simplifier turns that interval into a range predicate on `col`,
+ /// for example:
+ ///
+ /// ```text
+ /// date_trunc('month', ts) = '2024-05-01'
+ /// --> ts >= '2024-05-01' AND ts < '2024-06-01'
+ ///
+ /// date_trunc('hour', ts) = '2024-05-01T10:00:00'
+ /// --> ts >= '2024-05-01T10:00:00' AND ts < '2024-05-01T11:00:00'
+ ///
+ /// date_trunc('day', ts) > '2024-05-01'
+ /// --> ts >= '2024-05-02'
+ ///
+ /// date_trunc('year', ts) != '2024-01-01'
+ /// --> ts < '2024-01-01' OR ts >= '2025-01-01'
+ /// ```
+ ///
+ /// Returns [`PreimageResult::None`], leaving the predicate unchanged, when
+ /// no exact interval exists. See `timestamp_preimage_bounds` for the
+ /// conditions.
+ fn preimage(
+ &self,
+ args: &[Expr],
+ lit_expr: &Expr,
+ info: &SimplifyContext,
+ ) -> Result<PreimageResult> {
+ let [precision, col_expr] = take_function_args(self.name(), args)?;
+
+ // Granularity literal (e.g. "month").
+ let Some(granularity) = precision
+ .as_literal()
+ .and_then(|sv| sv.try_as_str().flatten())
+ .and_then(|s| parse_granularity(s).ok())
+ else {
+ return Ok(PreimageResult::None);
+ };
+
+ // Try to extract bounds for date columns
+ if let Some(result) = date_column_preimage(col_expr, lit_expr,
granularity, info)
+ {
+ return Ok(result);
+ }
+
+ // Try to extract bounds for time columns
+ if let Some(result) = time_column_preimage(col_expr, lit_expr,
granularity, info)
+ {
+ return Ok(result);
+ }
+
+ // The bounds must match the column's type
+ let target_type = info.get_data_type(col_expr)?;
+ let (unit, tz_opt) = match &target_type {
+ Timestamp(unit, tz) => (*unit, tz.clone()),
+ _ => return Ok(PreimageResult::None),
+ };
+
+ // timezone-aware buckets can have variable widths around DST.
+ // Stay conservative for those.
+ if tz_opt.is_some() && granularity.has_variable_width_in_tz() {
Review Comment:
I'd probably be worth making an exception for when the timezone is
`Some(Arc::from("UTC"))` or `Some(Arc::from("+00:00"))`. We know those aren't
actually going to be variable.
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -1433,6 +1434,143 @@ impl ScalarValue {
}
}
+ /// Returns a [`ScalarValue`] of type `target_type` representing `date`.
+ ///
+ /// For `Timestamp` types the result is midnight on `date` in the type's
+ /// timezone (or UTC when the type has no timezone).
+ ///
+ /// Returns `None` if `target_type` is not a date or timestamp type, or if
+ /// midnight on `date` cannot be represented such as when the timezone
skips
+ /// midnight that day (for example a daylight saving transition at 00:00).
+ ///
+ /// # Example
+ /// ```
+ /// # use std::sync::Arc;
+ /// # use arrow::datatypes::{DataType, TimeUnit};
+ /// # use chrono::NaiveDate;
+ /// # use datafusion_common::ScalarValue;
+ /// let date = NaiveDate::from_ymd_opt(2024, 5, 1).unwrap();
+ ///
+ /// // Dates
+ /// assert_eq!(
+ /// ScalarValue::try_from_date(date, &DataType::Date32),
+ /// Some(ScalarValue::Date32(Some(19844)))
+ /// );
+ ///
+ /// // Timestamps without a timezone are midnight UTC
+ /// assert_eq!(
+ /// ScalarValue::try_from_date(date,
&DataType::Timestamp(TimeUnit::Second, None)),
+ /// Some(ScalarValue::TimestampSecond(Some(1714521600), None))
+ /// );
+ ///
+ /// // Timestamps with a timezone are midnight in that timezone
+ /// let tz: Arc<str> = Arc::from("+02:00");
+ /// assert_eq!(
+ /// ScalarValue::try_from_date(
+ /// date,
+ /// &DataType::Timestamp(TimeUnit::Second, Some(Arc::clone(&tz)))
+ /// ),
+ /// Some(ScalarValue::TimestampSecond(Some(1714514400), Some(tz)))
+ /// );
+ ///
+ /// // Other types are not supported
+ /// assert_eq!(ScalarValue::try_from_date(date, &DataType::Int32), None);
+ /// ```
+ pub fn try_from_date(date: NaiveDate, target_type: &DataType) ->
Option<Self> {
+ Some(match target_type {
+ DataType::Date32 => {
+ ScalarValue::Date32(Some(Date32Type::from_naive_date(date)))
+ }
+ DataType::Date64 => {
+ ScalarValue::Date64(Some(Date64Type::from_naive_date(date)))
+ }
+ DataType::Timestamp(unit, tz_opt) => {
+ let naive_midnight = date.and_hms_opt(0, 0, 0)?;
+ let tz: Option<Tz> = tz_opt.clone().and_then(|s|
s.parse().ok());
+
+ // Midnight has no representation when the date falls outside
+ // the unit's range, or when the zone skips it.
Review Comment:
I don't know if it's worth pointing out that a date can only be out of range
if the unit is nanoseconds.
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -1433,6 +1434,143 @@ impl ScalarValue {
}
}
+ /// Returns a [`ScalarValue`] of type `target_type` representing `date`.
+ ///
+ /// For `Timestamp` types the result is midnight on `date` in the type's
+ /// timezone (or UTC when the type has no timezone).
+ ///
+ /// Returns `None` if `target_type` is not a date or timestamp type, or if
+ /// midnight on `date` cannot be represented such as when the timezone
skips
+ /// midnight that day (for example a daylight saving transition at 00:00).
+ ///
+ /// # Example
+ /// ```
+ /// # use std::sync::Arc;
+ /// # use arrow::datatypes::{DataType, TimeUnit};
+ /// # use chrono::NaiveDate;
+ /// # use datafusion_common::ScalarValue;
+ /// let date = NaiveDate::from_ymd_opt(2024, 5, 1).unwrap();
+ ///
+ /// // Dates
+ /// assert_eq!(
+ /// ScalarValue::try_from_date(date, &DataType::Date32),
+ /// Some(ScalarValue::Date32(Some(19844)))
+ /// );
+ ///
+ /// // Timestamps without a timezone are midnight UTC
+ /// assert_eq!(
+ /// ScalarValue::try_from_date(date,
&DataType::Timestamp(TimeUnit::Second, None)),
+ /// Some(ScalarValue::TimestampSecond(Some(1714521600), None))
+ /// );
+ ///
+ /// // Timestamps with a timezone are midnight in that timezone
+ /// let tz: Arc<str> = Arc::from("+02:00");
+ /// assert_eq!(
+ /// ScalarValue::try_from_date(
+ /// date,
+ /// &DataType::Timestamp(TimeUnit::Second, Some(Arc::clone(&tz)))
+ /// ),
+ /// Some(ScalarValue::TimestampSecond(Some(1714514400), Some(tz)))
+ /// );
+ ///
+ /// // Other types are not supported
+ /// assert_eq!(ScalarValue::try_from_date(date, &DataType::Int32), None);
+ /// ```
+ pub fn try_from_date(date: NaiveDate, target_type: &DataType) ->
Option<Self> {
+ Some(match target_type {
+ DataType::Date32 => {
+ ScalarValue::Date32(Some(Date32Type::from_naive_date(date)))
+ }
+ DataType::Date64 => {
+ ScalarValue::Date64(Some(Date64Type::from_naive_date(date)))
+ }
+ DataType::Timestamp(unit, tz_opt) => {
+ let naive_midnight = date.and_hms_opt(0, 0, 0)?;
+ let tz: Option<Tz> = tz_opt.clone().and_then(|s|
s.parse().ok());
+
+ // Midnight has no representation when the date falls outside
+ // the unit's range, or when the zone skips it.
+ let value = match unit {
Review Comment:
It seems odd to have two match blocks with identical arms one immediately
following the other. I wonder if they can be combined?
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -1433,6 +1434,143 @@ impl ScalarValue {
}
}
+ /// Returns a [`ScalarValue`] of type `target_type` representing `date`.
+ ///
+ /// For `Timestamp` types the result is midnight on `date` in the type's
+ /// timezone (or UTC when the type has no timezone).
+ ///
+ /// Returns `None` if `target_type` is not a date or timestamp type, or if
+ /// midnight on `date` cannot be represented such as when the timezone
skips
+ /// midnight that day (for example a daylight saving transition at 00:00).
+ ///
+ /// # Example
+ /// ```
+ /// # use std::sync::Arc;
+ /// # use arrow::datatypes::{DataType, TimeUnit};
+ /// # use chrono::NaiveDate;
+ /// # use datafusion_common::ScalarValue;
+ /// let date = NaiveDate::from_ymd_opt(2024, 5, 1).unwrap();
+ ///
+ /// // Dates
+ /// assert_eq!(
+ /// ScalarValue::try_from_date(date, &DataType::Date32),
+ /// Some(ScalarValue::Date32(Some(19844)))
+ /// );
+ ///
+ /// // Timestamps without a timezone are midnight UTC
+ /// assert_eq!(
+ /// ScalarValue::try_from_date(date,
&DataType::Timestamp(TimeUnit::Second, None)),
+ /// Some(ScalarValue::TimestampSecond(Some(1714521600), None))
+ /// );
+ ///
+ /// // Timestamps with a timezone are midnight in that timezone
+ /// let tz: Arc<str> = Arc::from("+02:00");
+ /// assert_eq!(
+ /// ScalarValue::try_from_date(
+ /// date,
+ /// &DataType::Timestamp(TimeUnit::Second, Some(Arc::clone(&tz)))
+ /// ),
+ /// Some(ScalarValue::TimestampSecond(Some(1714514400), Some(tz)))
+ /// );
+ ///
+ /// // Other types are not supported
+ /// assert_eq!(ScalarValue::try_from_date(date, &DataType::Int32), None);
+ /// ```
+ pub fn try_from_date(date: NaiveDate, target_type: &DataType) ->
Option<Self> {
+ Some(match target_type {
+ DataType::Date32 => {
+ ScalarValue::Date32(Some(Date32Type::from_naive_date(date)))
+ }
+ DataType::Date64 => {
+ ScalarValue::Date64(Some(Date64Type::from_naive_date(date)))
+ }
+ DataType::Timestamp(unit, tz_opt) => {
+ let naive_midnight = date.and_hms_opt(0, 0, 0)?;
+ let tz: Option<Tz> = tz_opt.clone().and_then(|s|
s.parse().ok());
+
+ // Midnight has no representation when the date falls outside
+ // the unit's range, or when the zone skips it.
+ let value = match unit {
+ TimeUnit::Second =>
TimestampSecondType::from_naive_datetime(
+ naive_midnight,
+ tz.as_ref(),
+ )?,
+ TimeUnit::Millisecond => {
+ TimestampMillisecondType::from_naive_datetime(
+ naive_midnight,
+ tz.as_ref(),
+ )?
+ }
+ TimeUnit::Microsecond => {
+ TimestampMicrosecondType::from_naive_datetime(
+ naive_midnight,
+ tz.as_ref(),
+ )?
+ }
+ TimeUnit::Nanosecond =>
TimestampNanosecondType::from_naive_datetime(
+ naive_midnight,
+ tz.as_ref(),
+ )?,
+ };
+ match unit {
+ TimeUnit::Second => {
+ ScalarValue::TimestampSecond(Some(value),
tz_opt.clone())
+ }
+ TimeUnit::Millisecond => {
+ ScalarValue::TimestampMillisecond(Some(value),
tz_opt.clone())
+ }
+ TimeUnit::Microsecond => {
+ ScalarValue::TimestampMicrosecond(Some(value),
tz_opt.clone())
+ }
+ TimeUnit::Nanosecond => {
+ ScalarValue::TimestampNanosecond(Some(value),
tz_opt.clone())
+ }
+ }
+ }
+ _ => return None,
+ })
+ }
+
+ /// Returns the raw `i64` value of a non-null timestamp with exactly the
Review Comment:
Is it worth mentioning that the value will always represent UTC offset
irrespective of the timezone being requested. This means the timezone is only
used for validating the type and does not effect the result value.
##########
datafusion/functions/src/datetime/date_trunc.rs:
##########
@@ -436,6 +452,97 @@ impl ScalarUDFImpl for DateTruncFunc {
&self.aliases
}
+ /// Compute the preimage for `date_trunc(granularity, col) = literal`.
+ ///
+ /// `date_trunc` floors a timestamp to the start of the given period, so
+ /// for any aligned literal `L` the preimage is the half-open interval
+ /// `[L, L + 1 granularity)`.
+ ///
+ /// The simplifier turns that interval into a range predicate on `col`,
+ /// for example:
+ ///
+ /// ```text
+ /// date_trunc('month', ts) = '2024-05-01'
+ /// --> ts >= '2024-05-01' AND ts < '2024-06-01'
+ ///
+ /// date_trunc('hour', ts) = '2024-05-01T10:00:00'
+ /// --> ts >= '2024-05-01T10:00:00' AND ts < '2024-05-01T11:00:00'
+ ///
+ /// date_trunc('day', ts) > '2024-05-01'
+ /// --> ts >= '2024-05-02'
+ ///
+ /// date_trunc('year', ts) != '2024-01-01'
+ /// --> ts < '2024-01-01' OR ts >= '2025-01-01'
+ /// ```
+ ///
+ /// Returns [`PreimageResult::None`], leaving the predicate unchanged, when
+ /// no exact interval exists. See `timestamp_preimage_bounds` for the
+ /// conditions.
+ fn preimage(
+ &self,
+ args: &[Expr],
+ lit_expr: &Expr,
+ info: &SimplifyContext,
+ ) -> Result<PreimageResult> {
+ let [precision, col_expr] = take_function_args(self.name(), args)?;
+
+ // Granularity literal (e.g. "month").
+ let Some(granularity) = precision
+ .as_literal()
+ .and_then(|sv| sv.try_as_str().flatten())
+ .and_then(|s| parse_granularity(s).ok())
+ else {
+ return Ok(PreimageResult::None);
+ };
+
+ // Try to extract bounds for date columns
+ if let Some(result) = date_column_preimage(col_expr, lit_expr,
granularity, info)
+ {
+ return Ok(result);
+ }
+
+ // Try to extract bounds for time columns
+ if let Some(result) = time_column_preimage(col_expr, lit_expr,
granularity, info)
+ {
+ return Ok(result);
+ }
+
+ // The bounds must match the column's type
+ let target_type = info.get_data_type(col_expr)?;
+ let (unit, tz_opt) = match &target_type {
+ Timestamp(unit, tz) => (*unit, tz.clone()),
+ _ => return Ok(PreimageResult::None),
+ };
+
+ // timezone-aware buckets can have variable widths around DST.
+ // Stay conservative for those.
Review Comment:
Months are variable in all time zones. Are they more variable in ones that
use DST?
--
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]
