Skip to content

Commit

Permalink
limit supported order and offset data types
Browse files Browse the repository at this point in the history
Signed-off-by: Richard Chien <[email protected]>
  • Loading branch information
stdrc committed Jan 24, 2024
1 parent e050e84 commit 2264345
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
8 changes: 8 additions & 0 deletions src/expr/core/src/window_function/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,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"),
}
Expand Down
13 changes: 12 additions & 1 deletion src/frontend/src/binder/expr/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,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(
Expand Down Expand Up @@ -665,7 +673,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
Expand Down

0 comments on commit 2264345

Please sign in to comment.