diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs index d0cb4263dbd9..3876fcf980b2 100644 --- a/datafusion/sql/src/expr/function.rs +++ b/datafusion/sql/src/expr/function.rs @@ -93,6 +93,8 @@ struct FunctionArgs { distinct: bool, /// WITHIN GROUP clause, if any within_group: Vec, + /// Was the function called without parenthesis, i.e. could this also be a column reference? + function_without_paranthesis: bool, } impl FunctionArgs { @@ -118,6 +120,7 @@ impl FunctionArgs { null_treatment, distinct: false, within_group, + function_without_paranthesis: matches!(args, FunctionArguments::None), }); }; @@ -199,6 +202,7 @@ impl FunctionArgs { null_treatment, distinct, within_group, + function_without_paranthesis: false, }) } } @@ -212,7 +216,7 @@ impl SqlToRel<'_, S> { ) -> Result { let function_args = FunctionArgs::try_new(function)?; let FunctionArgs { - name, + name: object_name, args, order_by, over, @@ -220,6 +224,7 @@ impl SqlToRel<'_, S> { null_treatment, distinct, within_group, + function_without_paranthesis, } = function_args; if over.is_some() && !within_group.is_empty() { @@ -235,18 +240,18 @@ impl SqlToRel<'_, S> { // it shouldn't have ordering requirement as function argument // required ordering should be defined in OVER clause. let is_function_window = over.is_some(); - let sql_parser_span = name.0[0].span(); - let name = if name.0.len() > 1 { + let sql_parser_span = object_name.0[0].span(); + let name = if object_name.0.len() > 1 { // DF doesn't handle compound identifiers // (e.g. "foo.bar") for function names yet - name.to_string() + object_name.to_string() } else { - match name.0[0].as_ident() { + match object_name.0[0].as_ident() { Some(ident) => crate::utils::normalize_ident(ident.clone()), None => { return plan_err!( "Expected an identifier in function name, but found {:?}", - name.0[0] + object_name.0[0] ) } } @@ -462,6 +467,31 @@ impl SqlToRel<'_, S> { ))); } } + + // workaround for https://github.com/apache/datafusion-sqlparser-rs/issues/1909 + if function_without_paranthesis { + let maybe_ids = object_name + .0 + .iter() + .map(|part| part.as_ident().cloned().ok_or(())) + .collect::, ()>>(); + if let Ok(ids) = maybe_ids { + if ids.len() == 1 { + return self.sql_identifier_to_expr( + ids.into_iter().next().unwrap(), + schema, + planner_context, + ); + } else { + return self.sql_compound_identifier_to_expr( + ids, + schema, + planner_context, + ); + } + } + } + // Could not find the relevant function, so return an error if let Some(suggested_func_name) = suggest_valid_function(&name, is_function_window, self.context_provider) diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index 1254a1997dbe..e92869873731 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -21,9 +21,8 @@ use datafusion_expr::planner::{ }; use sqlparser::ast::{ AccessExpr, BinaryOperator, CastFormat, CastKind, DataType as SQLDataType, - DictionaryField, Expr as SQLExpr, ExprWithAlias as SQLExprWithAlias, - FunctionArguments, MapEntry, StructField, Subscript, TrimWhereField, Value, - ValueWithSpan, + DictionaryField, Expr as SQLExpr, ExprWithAlias as SQLExprWithAlias, MapEntry, + StructField, Subscript, TrimWhereField, Value, ValueWithSpan, }; use datafusion_common::{ @@ -477,21 +476,7 @@ impl SqlToRel<'_, S> { ), SQLExpr::Function(function) => { - // workaround for https://github.com/apache/datafusion-sqlparser-rs/issues/1909 - if matches!(function.args, FunctionArguments::None) - && function.name.0.len() > 1 - && function.name.0.iter().all(|part| part.as_ident().is_some()) - { - let ids = function - .name - .0 - .iter() - .map(|part| part.as_ident().expect("just checked").clone()) - .collect(); - self.sql_compound_identifier_to_expr(ids, schema, planner_context) - } else { - self.sql_function_to_expr(function, schema, planner_context) - } + self.sql_function_to_expr(function, schema, planner_context) } SQLExpr::Rollup(exprs) => { diff --git a/datafusion/sqllogictest/test_files/select.slt b/datafusion/sqllogictest/test_files/select.slt index 9febf06b2510..109c2f209ad9 100644 --- a/datafusion/sqllogictest/test_files/select.slt +++ b/datafusion/sqllogictest/test_files/select.slt @@ -1875,10 +1875,57 @@ drop table t; # test "user" column # See https://github.com/apache/datafusion/issues/14141 statement count 0 -create table t_with_user(a int, user text) as values (1,'test'), (2,null); +create table t_with_user(a int, user text) as values (1,'test'), (2,null), (3,'foo'); query T select t_with_user.user from t_with_user; ---- test NULL +foo + +query IT +select * from t_with_user where t_with_user.user = 'foo'; +---- +3 foo + +query T +select user from t_with_user; +---- +test +NULL +foo + +query IT +select * from t_with_user where user = 'foo'; +---- +3 foo + +# test "current_time" column +# See https://github.com/apache/datafusion/issues/14141 +statement count 0 +create table t_with_current_time(a int, current_time text) as values (1,'now'), (2,null), (3,'later'); + +# here it's clear the the column was meant +query B +select t_with_current_time.current_time is not null from t_with_current_time; +---- +true +false +true + +# here it's the function +query B +select current_time is not null from t_with_current_time; +---- +true +true +true + +# and here it's the column again +query B +select "current_time" is not null from t_with_current_time; +---- +true +false +true