From d06f4ae510e3535c2f66c0975b9799cc6ad05866 Mon Sep 17 00:00:00 2001 From: Richard Chien Date: Tue, 23 Jan 2024 16:33:49 +0800 Subject: [PATCH] limit supported order and offset data types Signed-off-by: Richard Chien --- src/expr/core/src/window_function/call.rs | 8 ++++++++ src/frontend/src/binder/expr/function.rs | 13 ++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/expr/core/src/window_function/call.rs b/src/expr/core/src/window_function/call.rs index fe1a6db5fbba7..e5d81592a3d6d 100644 --- a/src/expr/core/src/window_function/call.rs +++ b/src/expr/core/src/window_function/call.rs @@ -334,6 +334,14 @@ impl FrameBoundsImpl for RangeFrameBounds { val ); } + if matches!(self.order_data_type, DataType::Timestamptz) { + // for `timestamptz`, we only support offset without `month` and `day` fields + if val.months() != 0 || val.days() != 0 { + bail!( + "for frame order column of type `timestamptz`, offset should not have non-zero `month` and `day`", + ); + } + } }, _ => unreachable!("other order column data types are not supported and should be banned in frontend"), } diff --git a/src/frontend/src/binder/expr/function.rs b/src/frontend/src/binder/expr/function.rs index 7ea7240db9893..3d2aba906a7db 100644 --- a/src/frontend/src/binder/expr/function.rs +++ b/src/frontend/src/binder/expr/function.rs @@ -634,7 +634,15 @@ impl Binder { // NOTE: actually in PG it can be a larger type, but we don't support this here t @ data_types::range_frame_numeric!() => t.clone(), // for datetime ordering columns, `offset` should be interval - data_types::range_frame_datetime!() => DataType::Interval, + t @ data_types::range_frame_datetime!() => { + if matches!(t, DataType::Date | DataType::Time) { + bail_not_implemented!( + "`RANGE` frame with offset of type `{}` is not implemented yet, please manually cast the `ORDER BY` column to `timestamp`", + t + ); + } + DataType::Interval + } // other types are not supported t => { return Err(ErrorCode::NotSupported( @@ -668,7 +676,10 @@ impl Binder { ); } }; + + // Validate the frame bounds, may return `ExprError` to user if the bounds given are not valid. bounds.validate()?; + Some(Frame { bounds, exclusion }) } else { None