From d9372a358dbfdc48fd8367030139d1b215f32756 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Tue, 14 Mar 2023 17:54:09 +0800 Subject: [PATCH 1/2] fix(expr): do not construct error for extracting time subfield Signed-off-by: Bugen Zhao --- src/expr/src/vector_op/extract.rs | 57 ++++++++++++++++--------------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/src/expr/src/vector_op/extract.rs b/src/expr/src/vector_op/extract.rs index 3122fe7883c2..0aad0fb42491 100644 --- a/src/expr/src/vector_op/extract.rs +++ b/src/expr/src/vector_op/extract.rs @@ -15,61 +15,64 @@ use chrono::{Datelike, Timelike}; use risingwave_common::types::{Decimal, NaiveDateTimeWrapper, NaiveDateWrapper, NaiveTimeWrapper}; -use crate::{bail, Result}; +use crate::{ExprError, Result}; -fn extract_time(time: T, time_unit: &str) -> Result +fn extract_time(time: T, field: &str) -> Option where T: Timelike, { - match time_unit { - "HOUR" => Ok(time.hour().into()), - "MINUTE" => Ok(time.minute().into()), - "SECOND" => Ok(time.second().into()), - _ => bail!("Unsupported time unit {} in extract function", time_unit), - } + Some(match field { + "HOUR" => time.hour().into(), + "MINUTE" => time.minute().into(), + "SECOND" => time.second().into(), + _ => return None, + }) } -fn extract_date(date: T, time_unit: &str) -> Result +fn extract_date(date: T, field: &str) -> Option where T: Datelike, { - match time_unit { - "DAY" => Ok(date.day().into()), - "MONTH" => Ok(date.month().into()), - "YEAR" => Ok(date.year().into()), + Some(match field { + "DAY" => date.day().into(), + "MONTH" => date.month().into(), + "YEAR" => date.year().into(), // Sun = 0 and Sat = 6 - "DOW" => Ok(date.weekday().num_days_from_sunday().into()), - "DOY" => Ok(date.ordinal().into()), - _ => bail!("Unsupported time unit {} in extract function", time_unit), + "DOW" => date.weekday().num_days_from_sunday().into(), + "DOY" => date.ordinal().into(), + _ => return None, + }) +} + +fn invalid_unit(name: &'static str, unit: &str) -> ExprError { + ExprError::InvalidParam { + name, + reason: format!("\"{unit}\" not recognized or supported"), } } pub fn extract_from_date(time_unit: &str, date: NaiveDateWrapper) -> Result { - extract_date(date.0, time_unit) + extract_date(date.0, time_unit).ok_or_else(|| invalid_unit("date unit", time_unit)) } pub fn extract_from_timestamp(time_unit: &str, timestamp: NaiveDateTimeWrapper) -> Result { let time = timestamp.0; - let mut res = extract_date(time, time_unit); - if res.is_err() { - res = extract_time(time, time_unit); - } - res + + extract_date(time, time_unit) + .or_else(|| extract_time(time, time_unit)) + .ok_or_else(|| invalid_unit("timestamp unit", time_unit)) } pub fn extract_from_timestamptz(time_unit: &str, usecs: i64) -> Result { match time_unit { "EPOCH" => Ok(Decimal::from(usecs) / 1_000_000.into()), // TODO(#5826): all other units depend on implicit session TimeZone - _ => bail!( - "Unsupported timestamp with time zone unit {} in extract function", - time_unit - ), + _ => Err(invalid_unit("timestamp with time zone units", time_unit)), } } pub fn extract_from_time(time_unit: &str, time: NaiveTimeWrapper) -> Result { - extract_time(time.0, time_unit) + extract_time(time.0, time_unit).ok_or_else(|| invalid_unit("time unit", time_unit)) } #[cfg(test)] From de1943da61eaac56886cd21fe81c0679749f5ae3 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Tue, 14 Mar 2023 17:55:17 +0800 Subject: [PATCH 2/2] rename parameter Signed-off-by: Bugen Zhao --- src/expr/src/vector_op/extract.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/expr/src/vector_op/extract.rs b/src/expr/src/vector_op/extract.rs index 0aad0fb42491..a8605b36ba0e 100644 --- a/src/expr/src/vector_op/extract.rs +++ b/src/expr/src/vector_op/extract.rs @@ -17,11 +17,11 @@ use risingwave_common::types::{Decimal, NaiveDateTimeWrapper, NaiveDateWrapper, use crate::{ExprError, Result}; -fn extract_time(time: T, field: &str) -> Option +fn extract_time(time: T, unit: &str) -> Option where T: Timelike, { - Some(match field { + Some(match unit { "HOUR" => time.hour().into(), "MINUTE" => time.minute().into(), "SECOND" => time.second().into(), @@ -29,11 +29,11 @@ where }) } -fn extract_date(date: T, field: &str) -> Option +fn extract_date(date: T, unit: &str) -> Option where T: Datelike, { - Some(match field { + Some(match unit { "DAY" => date.day().into(), "MONTH" => date.month().into(), "YEAR" => date.year().into(), @@ -51,28 +51,28 @@ fn invalid_unit(name: &'static str, unit: &str) -> ExprError { } } -pub fn extract_from_date(time_unit: &str, date: NaiveDateWrapper) -> Result { - extract_date(date.0, time_unit).ok_or_else(|| invalid_unit("date unit", time_unit)) +pub fn extract_from_date(unit: &str, date: NaiveDateWrapper) -> Result { + extract_date(date.0, unit).ok_or_else(|| invalid_unit("date unit", unit)) } -pub fn extract_from_timestamp(time_unit: &str, timestamp: NaiveDateTimeWrapper) -> Result { +pub fn extract_from_timestamp(unit: &str, timestamp: NaiveDateTimeWrapper) -> Result { let time = timestamp.0; - extract_date(time, time_unit) - .or_else(|| extract_time(time, time_unit)) - .ok_or_else(|| invalid_unit("timestamp unit", time_unit)) + extract_date(time, unit) + .or_else(|| extract_time(time, unit)) + .ok_or_else(|| invalid_unit("timestamp unit", unit)) } -pub fn extract_from_timestamptz(time_unit: &str, usecs: i64) -> Result { - match time_unit { +pub fn extract_from_timestamptz(unit: &str, usecs: i64) -> Result { + match unit { "EPOCH" => Ok(Decimal::from(usecs) / 1_000_000.into()), // TODO(#5826): all other units depend on implicit session TimeZone - _ => Err(invalid_unit("timestamp with time zone units", time_unit)), + _ => Err(invalid_unit("timestamp with time zone units", unit)), } } -pub fn extract_from_time(time_unit: &str, time: NaiveTimeWrapper) -> Result { - extract_time(time.0, time_unit).ok_or_else(|| invalid_unit("time unit", time_unit)) +pub fn extract_from_time(unit: &str, time: NaiveTimeWrapper) -> Result { + extract_time(time.0, unit).ok_or_else(|| invalid_unit("time unit", unit)) } #[cfg(test)]