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/11/01 16:19:56 UTC
(arrow-datafusion) branch main updated: Minor: error on unsupported RESPECT NULLs syntax (#7998)
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 7788b90694 Minor: error on unsupported RESPECT NULLs syntax (#7998)
7788b90694 is described below
commit 7788b90694dc57fb88cc5a6da8cbfd024077e05b
Author: Andrew Lamb <an...@nerdnetworks.org>
AuthorDate: Wed Nov 1 12:19:50 2023 -0400
Minor: error on unsupported RESPECT NULLs syntax (#7998)
* Minor: error on unsupported RESPECT NULLs syntax
* fix clippy
* Update datafusion/sql/tests/sql_integration.rs
Co-authored-by: Liang-Chi Hsieh <vi...@gmail.com>
---------
Co-authored-by: Liang-Chi Hsieh <vi...@gmail.com>
---
datafusion/sql/src/expr/function.rs | 68 ++++++++++++++++-----------------
datafusion/sql/tests/sql_integration.rs | 10 +++++
2 files changed, 43 insertions(+), 35 deletions(-)
diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs
index 8af5fef84a..c58b8319ce 100644
--- a/datafusion/sql/src/expr/function.rs
+++ b/datafusion/sql/src/expr/function.rs
@@ -36,44 +36,57 @@ use super::arrow_cast::ARROW_CAST_NAME;
impl<'a, S: ContextProvider> SqlToRel<'a, S> {
pub(super) fn sql_function_to_expr(
&self,
- mut function: SQLFunction,
+ function: SQLFunction,
schema: &DFSchema,
planner_context: &mut PlannerContext,
) -> Result<Expr> {
- let name = if function.name.0.len() > 1 {
+ let SQLFunction {
+ name,
+ args,
+ over,
+ distinct,
+ filter,
+ null_treatment,
+ special: _, // true if not called with trailing parens
+ order_by,
+ } = function;
+
+ if let Some(null_treatment) = null_treatment {
+ return not_impl_err!("Null treatment in aggregate functions is not supported: {null_treatment}");
+ }
+
+ let name = if name.0.len() > 1 {
// DF doesn't handle compound identifiers
// (e.g. "foo.bar") for function names yet
- function.name.to_string()
+ name.to_string()
} else {
- crate::utils::normalize_ident(function.name.0[0].clone())
+ crate::utils::normalize_ident(name.0[0].clone())
};
// user-defined function (UDF) should have precedence in case it has the same name as a scalar built-in function
if let Some(fm) = self.context_provider.get_function_meta(&name) {
- let args =
- self.function_args_to_expr(function.args, schema, planner_context)?;
+ let args = self.function_args_to_expr(args, schema, planner_context)?;
return Ok(Expr::ScalarUDF(ScalarUDF::new(fm, args)));
}
// next, scalar built-in
if let Ok(fun) = BuiltinScalarFunction::from_str(&name) {
- let args =
- self.function_args_to_expr(function.args, schema, planner_context)?;
+ let args = self.function_args_to_expr(args, schema, planner_context)?;
return Ok(Expr::ScalarFunction(ScalarFunction::new(fun, args)));
};
// If function is a window function (it has an OVER clause),
// it shouldn't have ordering requirement as function argument
// required ordering should be defined in OVER clause.
- let is_function_window = function.over.is_some();
- if !function.order_by.is_empty() && is_function_window {
+ let is_function_window = over.is_some();
+ if !order_by.is_empty() && is_function_window {
return plan_err!(
"Aggregate ORDER BY is not implemented for window functions"
);
}
// then, window function
- if let Some(WindowType::WindowSpec(window)) = function.over.take() {
+ if let Some(WindowType::WindowSpec(window)) = over {
let partition_by = window
.partition_by
.into_iter()
@@ -97,11 +110,8 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
if let Ok(fun) = self.find_window_func(&name) {
let expr = match fun {
WindowFunction::AggregateFunction(aggregate_fun) => {
- let args = self.function_args_to_expr(
- function.args,
- schema,
- planner_context,
- )?;
+ let args =
+ self.function_args_to_expr(args, schema, planner_context)?;
Expr::WindowFunction(expr::WindowFunction::new(
WindowFunction::AggregateFunction(aggregate_fun),
@@ -113,11 +123,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
}
_ => Expr::WindowFunction(expr::WindowFunction::new(
fun,
- self.function_args_to_expr(
- function.args,
- schema,
- planner_context,
- )?,
+ self.function_args_to_expr(args, schema, planner_context)?,
partition_by,
order_by,
window_frame,
@@ -128,8 +134,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
} else {
// User defined aggregate functions (UDAF) have precedence in case it has the same name as a scalar built-in function
if let Some(fm) = self.context_provider.get_aggregate_meta(&name) {
- let args =
- self.function_args_to_expr(function.args, schema, planner_context)?;
+ let args = self.function_args_to_expr(args, schema, planner_context)?;
return Ok(Expr::AggregateUDF(expr::AggregateUDF::new(
fm, args, None, None,
)));
@@ -137,17 +142,11 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
// next, aggregate built-ins
if let Ok(fun) = AggregateFunction::from_str(&name) {
- let distinct = function.distinct;
- let order_by = self.order_by_to_sort_expr(
- &function.order_by,
- schema,
- planner_context,
- )?;
+ let order_by =
+ self.order_by_to_sort_expr(&order_by, schema, planner_context)?;
let order_by = (!order_by.is_empty()).then_some(order_by);
- let args =
- self.function_args_to_expr(function.args, schema, planner_context)?;
- let filter: Option<Box<Expr>> = function
- .filter
+ let args = self.function_args_to_expr(args, schema, planner_context)?;
+ let filter: Option<Box<Expr>> = filter
.map(|e| self.sql_expr_to_logical_expr(*e, schema, planner_context))
.transpose()?
.map(Box::new);
@@ -159,8 +158,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
// Special case arrow_cast (as its type is dependent on its argument value)
if name == ARROW_CAST_NAME {
- let args =
- self.function_args_to_expr(function.args, schema, planner_context)?;
+ let args = self.function_args_to_expr(args, schema, planner_context)?;
return super::arrow_cast::create_arrow_cast(args, schema);
}
}
diff --git a/datafusion/sql/tests/sql_integration.rs b/datafusion/sql/tests/sql_integration.rs
index 2446ee0a58..ff6dca7eef 100644
--- a/datafusion/sql/tests/sql_integration.rs
+++ b/datafusion/sql/tests/sql_integration.rs
@@ -1287,6 +1287,16 @@ fn select_simple_aggregate_repeated_aggregate_with_unique_aliases() {
);
}
+#[test]
+fn select_simple_aggregate_respect_nulls() {
+ let sql = "SELECT MIN(age) RESPECT NULLS FROM person";
+ let err = logical_plan(sql).expect_err("query should have failed");
+
+ assert_contains!(
+ err.strip_backtrace(),
+ "This feature is not implemented: Null treatment in aggregate functions is not supported: RESPECT NULLS"
+ );
+}
#[test]
fn select_from_typed_string_values() {
quick_test(