You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by al...@apache.org on 2023/06/01 19:55:23 UTC
[arrow-datafusion] branch main updated: minor: make window frame error messages more consistent (#6519)
This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 9333e3f1b4 minor: make window frame error messages more consistent (#6519)
9333e3f1b4 is described below
commit 9333e3f1b4ef86cb8f4cfde1d37c2ace50da30aa
Author: comphead <co...@users.noreply.github.com>
AuthorDate: Thu Jun 1 12:55:15 2023 -0700
minor: make window frame error messages more consistent (#6519)
---
.../core/tests/sqllogictests/test_files/window.slt | 6 +--
datafusion/expr/src/window_frame.rs | 43 ++++++++++------------
2 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/datafusion/core/tests/sqllogictests/test_files/window.slt b/datafusion/core/tests/sqllogictests/test_files/window.slt
index 8ab9b29da4..f1280fa688 100644
--- a/datafusion/core/tests/sqllogictests/test_files/window.slt
+++ b/datafusion/core/tests/sqllogictests/test_files/window.slt
@@ -920,7 +920,7 @@ drop table temp
#fn window_frame_ranges_unbounded_preceding_err
-statement error DataFusion error: Error during planning: Invalid window frame: end bound cannot be unbounded preceding
+statement error DataFusion error: Error during planning: Invalid window frame: end bound cannot be UNBOUNDED PRECEDING
SELECT
SUM(c2) OVER (ORDER BY c2 RANGE BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED PRECEDING),
COUNT(*) OVER (ORDER BY c2 RANGE BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED PRECEDING)
@@ -1036,7 +1036,7 @@ LIMIT 10
#fn window_frame_groups_without_order_by
# Try executing an erroneous query (the ORDER BY clause is missing in the
# window frame):
-statement error Error during planning: GROUPS mode requires an ORDER BY clause
+statement error Error during planning: GROUPS requires an ORDER BY clause
SELECT
SUM(c4) OVER(PARTITION BY c2 GROUPS BETWEEN 1 PRECEDING AND 1 FOLLOWING)
FROM aggregate_test_100
@@ -1061,7 +1061,7 @@ SELECT
COUNT(c1) OVER (ORDER BY c2 RANGE BETWEEN 2 FOLLOWING AND 1 FOLLOWING)
FROM aggregate_test_100
-statement error DataFusion error: Error during planning: GROUPS mode requires an ORDER BY clause
+statement error DataFusion error: Error during planning: GROUPS requires an ORDER BY clause
SELECT
COUNT(c1) OVER(GROUPS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING)
FROM aggregate_test_100
diff --git a/datafusion/expr/src/window_frame.rs b/datafusion/expr/src/window_frame.rs
index f48dd33930..b2d1882788 100644
--- a/datafusion/expr/src/window_frame.rs
+++ b/datafusion/expr/src/window_frame.rs
@@ -68,17 +68,15 @@ impl TryFrom<ast::WindowFrame> for WindowFrame {
if let WindowFrameBound::Following(val) = &start_bound {
if val.is_null() {
- return Err(DataFusionError::Plan(
- "Invalid window frame: start bound cannot be unbounded following"
- .to_owned(),
- ));
+ plan_error(
+ "Invalid window frame: start bound cannot be UNBOUNDED FOLLOWING",
+ )?
}
} else if let WindowFrameBound::Preceding(val) = &end_bound {
if val.is_null() {
- return Err(DataFusionError::Plan(
- "Invalid window frame: end bound cannot be unbounded preceding"
- .to_owned(),
- ));
+ plan_error(
+ "Invalid window frame: end bound cannot be UNBOUNDED PRECEDING",
+ )?
}
};
Ok(Self {
@@ -163,13 +161,10 @@ pub fn regularize(mut frame: WindowFrame, order_bys: usize) -> Result<WindowFram
frame.end_bound = WindowFrameBound::Following(ScalarValue::UInt64(None));
}
} else {
- return Err(DataFusionError::Plan(format!(
- "With window frame of type RANGE, the ORDER BY expression must be of length 1, got {order_bys}")));
+ plan_error("RANGE requires exactly one ORDER BY column")?
}
} else if frame.units == WindowFrameUnits::Groups && order_bys == 0 {
- return Err(DataFusionError::Plan(
- "GROUPS mode requires an ORDER BY clause".to_string(),
- ));
+ plan_error("GROUPS requires an ORDER BY clause")?
};
Ok(frame)
}
@@ -246,8 +241,9 @@ pub fn convert_frame_bound_to_scalar_value(v: ast::Expr) -> Result<ScalarValue>
let result = match *value {
ast::Expr::Value(ast::Value::SingleQuotedString(item)) => item,
e => {
- let msg = format!("INTERVAL expression cannot be {e:?}");
- return Err(DataFusionError::SQL(ParserError(msg)));
+ return Err(DataFusionError::SQL(ParserError(format!(
+ "INTERVAL expression cannot be {e:?}"
+ ))));
}
};
if let Some(leading_field) = leading_field {
@@ -256,15 +252,16 @@ pub fn convert_frame_bound_to_scalar_value(v: ast::Expr) -> Result<ScalarValue>
result
}
}
- _ => {
- return Err(DataFusionError::Plan(
- "Invalid window frame: frame offsets must be non negative integers"
- .to_owned(),
- ));
- }
+ _ => plan_error(
+ "Invalid window frame: frame offsets must be non negative integers",
+ )?,
})))
}
+fn plan_error<T>(err_message: &str) -> Result<T> {
+ Err(DataFusionError::Plan(err_message.to_string()))
+}
+
impl fmt::Display for WindowFrameBound {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
@@ -340,7 +337,7 @@ mod tests {
let err = WindowFrame::try_from(window_frame).unwrap_err();
assert_eq!(
err.to_string(),
- "Error during planning: Invalid window frame: start bound cannot be unbounded following".to_owned()
+ "Error during planning: Invalid window frame: start bound cannot be UNBOUNDED FOLLOWING".to_owned()
);
let window_frame = ast::WindowFrame {
@@ -351,7 +348,7 @@ mod tests {
let err = WindowFrame::try_from(window_frame).unwrap_err();
assert_eq!(
err.to_string(),
- "Error during planning: Invalid window frame: end bound cannot be unbounded preceding".to_owned()
+ "Error during planning: Invalid window frame: end bound cannot be UNBOUNDED PRECEDING".to_owned()
);
let window_frame = ast::WindowFrame {