diff --git a/src/common/src/types/interval.rs b/src/common/src/types/interval.rs index 9224c8955db66..9935c2870ad28 100644 --- a/src/common/src/types/interval.rs +++ b/src/common/src/types/interval.rs @@ -384,6 +384,11 @@ impl Interval { self > &Self::from_month_day_usec(0, 0, 0) } + /// Checks if all fields of [`Interval`] are all non-negative. + pub fn is_never_negative(&self) -> bool { + self.months >= 0 && self.days >= 0 && self.usecs >= 0 + } + /// Truncate the interval to the precision of milliseconds. /// /// # Example diff --git a/src/common/src/types/mod.rs b/src/common/src/types/mod.rs index fe27f54967a16..7535cdddd4f3c 100644 --- a/src/common/src/types/mod.rs +++ b/src/common/src/types/mod.rs @@ -295,29 +295,32 @@ impl From for PbTypeName { /// Convenient macros to generate match arms for [`DataType`](crate::types::DataType). pub mod data_types { - /// Numeric [`DataType`](crate::types::DataType)s. + /// Numeric [`DataType`](crate::types::DataType)s supported to be `offset` of `RANGE` frame. #[macro_export] - macro_rules! numeric { + macro_rules! range_frame_numeric { () => { DataType::Int16 | DataType::Int32 | DataType::Int64 - | DataType::Serial | DataType::Float32 | DataType::Float64 | DataType::Decimal }; } - pub use numeric; + pub use range_frame_numeric; - /// Integer [`DataType`](crate::types::DataType)s. + /// Integer [`DataType`](crate::types::DataType)s supported to be `offset` of `RANGE` frame. #[macro_export] - macro_rules! integer { + macro_rules! range_frame_datetime { () => { - DataType::Int16 | DataType::Int32 | DataType::Int64 + DataType::Date + | DataType::Time + | DataType::Timestamp + | DataType::Timestamptz + | DataType::Interval }; } - pub use integer; + pub use range_frame_datetime; } impl DataType { @@ -373,7 +376,16 @@ impl DataType { } pub fn is_numeric(&self) -> bool { - matches!(self, data_types::numeric!()) + matches!( + self, + DataType::Int16 + | DataType::Int32 + | DataType::Int64 + | DataType::Serial + | DataType::Float32 + | DataType::Float64 + | DataType::Decimal + ) } pub fn is_scalar(&self) -> bool { @@ -389,7 +401,7 @@ impl DataType { } pub fn is_int(&self) -> bool { - matches!(self, data_types::integer!()) + matches!(self, DataType::Int16 | DataType::Int32 | DataType::Int64) } /// Returns the output type of time window function on a given input type. diff --git a/src/expr/core/src/window_function/call.rs b/src/expr/core/src/window_function/call.rs index 3896ddf3e22e5..558e7af1324cf 100644 --- a/src/expr/core/src/window_function/call.rs +++ b/src/expr/core/src/window_function/call.rs @@ -17,7 +17,7 @@ use std::fmt::Display; use enum_as_inner::EnumAsInner; use parse_display::Display; use risingwave_common::bail; -use risingwave_common::types::{DataType, ScalarImpl, ScalarRefImpl, ToText}; +use risingwave_common::types::{DataType, IsNegative, ScalarImpl, ScalarRefImpl, ToText}; use risingwave_pb::expr::window_frame::{PbBound, PbExclusion}; use risingwave_pb::expr::{PbWindowFrame, PbWindowFunction}; use FrameBound::{CurrentRow, Following, Preceding, UnboundedFollowing, UnboundedPreceding}; @@ -198,25 +198,33 @@ impl Display for RangeFrameBounds { impl FrameBoundsImpl for RangeFrameBounds { fn validate(&self) -> Result<()> { + fn validate_non_negative(val: impl IsNegative + Display) -> Result<()> { + if val.is_negative() { + bail!( + "frame bound offset should be non-negative, but {} is given", + val + ); + } + Ok(()) + } + FrameBound::validate_bounds(&self.start, &self.end, |offset| { match offset.as_scalar_ref_impl() { - // TODO(): use decl macro to merge with the following - ScalarRefImpl::Int16(val) => { - if val < 0 { - bail!("frame bound offset should be non-negative, but {} is given", val); - } - } - ScalarRefImpl::Int32(val) => { - if val < 0 { - bail!("frame bound offset should be non-negative, but {} is given", val); - } - } - ScalarRefImpl::Int64(val) => { - if val < 0 { - bail!("frame bound offset should be non-negative, but {} is given", val); + // TODO(rc): use decl macro? + ScalarRefImpl::Int16(val) => validate_non_negative(val)?, + ScalarRefImpl::Int32(val) => validate_non_negative(val)?, + ScalarRefImpl::Int64(val) => validate_non_negative(val)?, + ScalarRefImpl::Float32(val) => validate_non_negative(val)?, + ScalarRefImpl::Float64(val) => validate_non_negative(val)?, + ScalarRefImpl::Decimal(val) => validate_non_negative(val)?, + ScalarRefImpl::Interval(val) => { + if !val.is_never_negative() { + bail!( + "for frame bound offset of type `interval`, each field should be non-negative, but {} is given", + val + ); } - } - // TODO(): datetime types + }, _ => unreachable!("other order column data types are not supported and should be banned in frontend"), } Ok(()) diff --git a/src/frontend/src/binder/expr/function.rs b/src/frontend/src/binder/expr/function.rs index 0c7126fb98db1..137cb02a8745a 100644 --- a/src/frontend/src/binder/expr/function.rs +++ b/src/frontend/src/binder/expr/function.rs @@ -697,18 +697,14 @@ impl Binder { let offset_data_type = match order_data_type { // for numeric ordering columns, `offset` should be the same type // NOTE: actually in PG it can be a larger type, but we don't support this here - t @ data_types::numeric!() => t, + t @ data_types::range_frame_numeric!() => t, // for datetime ordering columns, `offset` should be interval - DataType::Date - | DataType::Time - | DataType::Timestamp - | DataType::Timestamptz - | DataType::Interval => DataType::Interval, + data_types::range_frame_datetime!() => DataType::Interval, // other types are not supported t => { return Err(ErrorCode::NotSupported( format!( - "`RANGE` frame with offset of `{}` type is not supported", + "`RANGE` frame with offset of type `{}` is not supported", t ), "Please re-consider the `ORDER BY` column".to_string(),