alamb commented on code in PR #4516:
URL: https://github.com/apache/arrow-datafusion/pull/4516#discussion_r1039903041
##########
datafusion/core/tests/sql/window.rs:
##########
@@ -53,25 +53,25 @@ async fn csv_query_window_with_partition_by() -> Result<()>
{
register_aggregate_csv(&ctx).await?;
let sql = "select \
c9, \
Review Comment:
these sure look nicer -- BTW if you are looking for easier to maintain
tests, you may want to check out the new sqllogictests runner we are working on
https://github.com/apache/arrow-datafusion/pull/4505
##########
datafusion/expr/src/window_frame.rs:
##########
@@ -66,33 +66,51 @@ impl TryFrom<ast::WindowFrame> for WindowFrame {
None => WindowFrameBound::CurrentRow,
};
- if let WindowFrameBound::Following(ScalarValue::Utf8(None)) =
start_bound {
- Err(DataFusionError::Execution(
- "Invalid window frame: start bound cannot be unbounded
following"
- .to_owned(),
- ))
- } else if let WindowFrameBound::Preceding(ScalarValue::Utf8(None)) =
end_bound {
- Err(DataFusionError::Execution(
- "Invalid window frame: end bound cannot be unbounded preceding"
- .to_owned(),
- ))
- } else {
- let units = value.units.into();
- Ok(Self {
- units,
- start_bound,
- end_bound,
- })
- }
+ if let WindowFrameBound::Following(val) = &start_bound {
+ if val.is_null() {
+ return Err(DataFusionError::Execution(
+ "Invalid window frame: start bound cannot be unbounded
following"
+ .to_owned(),
+ ));
+ }
+ } else if let WindowFrameBound::Preceding(val) = &end_bound {
+ if val.is_null() {
+ return Err(DataFusionError::Execution(
+ "Invalid window frame: end bound cannot be unbounded
preceding"
+ .to_owned(),
+ ));
+ }
+ };
+ Ok(Self {
+ units: value.units.into(),
+ start_bound,
+ end_bound,
+ })
}
}
-impl Default for WindowFrame {
- fn default() -> Self {
- WindowFrame {
- units: WindowFrameUnits::Range,
- start_bound: WindowFrameBound::Preceding(ScalarValue::Utf8(None)),
- end_bound: WindowFrameBound::CurrentRow,
+impl WindowFrame {
+ /// Creates a new, default window frame (with the meaning of default
depending on whether the
Review Comment:
Thank you for the clear comments 👍
--
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]
