Skip to content

Commit e8c9ff0

Browse files
crepererumThinkRedstone
authored andcommitted
fix: try to lower plain reserved functions to columns as well (apache#16669)
When a reserved function like `user` is called without parenthesis, it may as well be a column. This works in PostgreSQL for example: ```text psql (17.5 (Debian 17.5-1.pgdg120+1)) Type "help" for help. postgres=# create table t(a int, "user" text); CREATE TABLE postgres=# insert into t values (1, 'foo'); INSERT 0 1 postgres=# select t.user from t; user ------ foo (1 row) postgres=# select user from t; user ---------- postgres (1 row) ``` However sqlparser tries to detect these functions, see apache/datafusion-sqlparser-rs#1909 We now first try to use the respective function and then also consider columns. Fixes apache#14141.
1 parent a7d17e4 commit e8c9ff0

File tree

2 files changed

+94
-6
lines changed

2 files changed

+94
-6
lines changed

datafusion/sql/src/expr/function.rs

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ struct FunctionArgs {
9393
distinct: bool,
9494
/// WITHIN GROUP clause, if any
9595
within_group: Vec<OrderByExpr>,
96+
/// Was the function called without parenthesis, i.e. could this also be a column reference?
97+
function_without_paranthesis: bool,
9698
}
9799

98100
impl FunctionArgs {
@@ -118,6 +120,7 @@ impl FunctionArgs {
118120
null_treatment,
119121
distinct: false,
120122
within_group,
123+
function_without_paranthesis: matches!(args, FunctionArguments::None),
121124
});
122125
};
123126

@@ -199,6 +202,7 @@ impl FunctionArgs {
199202
null_treatment,
200203
distinct,
201204
within_group,
205+
function_without_paranthesis: false,
202206
})
203207
}
204208
}
@@ -212,14 +216,15 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
212216
) -> Result<Expr> {
213217
let function_args = FunctionArgs::try_new(function)?;
214218
let FunctionArgs {
215-
name,
219+
name: object_name,
216220
args,
217221
order_by,
218222
over,
219223
filter,
220224
null_treatment,
221225
distinct,
222226
within_group,
227+
function_without_paranthesis,
223228
} = function_args;
224229

225230
if over.is_some() && !within_group.is_empty() {
@@ -231,18 +236,18 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
231236
// it shouldn't have ordering requirement as function argument
232237
// required ordering should be defined in OVER clause.
233238
let is_function_window = over.is_some();
234-
let sql_parser_span = name.0[0].span();
235-
let name = if name.0.len() > 1 {
239+
let sql_parser_span = object_name.0[0].span();
240+
let name = if object_name.0.len() > 1 {
236241
// DF doesn't handle compound identifiers
237242
// (e.g. "foo.bar") for function names yet
238-
name.to_string()
243+
object_name.to_string()
239244
} else {
240-
match name.0[0].as_ident() {
245+
match object_name.0[0].as_ident() {
241246
Some(ident) => crate::utils::normalize_ident(ident.clone()),
242247
None => {
243248
return plan_err!(
244249
"Expected an identifier in function name, but found {:?}",
245-
name.0[0]
250+
object_name.0[0]
246251
)
247252
}
248253
}
@@ -453,6 +458,31 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
453458
)));
454459
}
455460
}
461+
462+
// workaround for https://github.com/apache/datafusion-sqlparser-rs/issues/1909
463+
if function_without_paranthesis {
464+
let maybe_ids = object_name
465+
.0
466+
.iter()
467+
.map(|part| part.as_ident().cloned().ok_or(()))
468+
.collect::<Result<Vec<_>, ()>>();
469+
if let Ok(ids) = maybe_ids {
470+
if ids.len() == 1 {
471+
return self.sql_identifier_to_expr(
472+
ids.into_iter().next().unwrap(),
473+
schema,
474+
planner_context,
475+
);
476+
} else {
477+
return self.sql_compound_identifier_to_expr(
478+
ids,
479+
schema,
480+
planner_context,
481+
);
482+
}
483+
}
484+
}
485+
456486
// Could not find the relevant function, so return an error
457487
if let Some(suggested_func_name) =
458488
suggest_valid_function(&name, is_function_window, self.context_provider)

datafusion/sqllogictest/test_files/select.slt

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1871,3 +1871,61 @@ select *, count(*) over() as ta from t;
18711871

18721872
statement count 0
18731873
drop table t;
1874+
1875+
# test "user" column
1876+
# See https://github.com/apache/datafusion/issues/14141
1877+
statement count 0
1878+
create table t_with_user(a int, user text) as values (1,'test'), (2,null), (3,'foo');
1879+
1880+
query T
1881+
select t_with_user.user from t_with_user;
1882+
----
1883+
test
1884+
NULL
1885+
foo
1886+
1887+
query IT
1888+
select * from t_with_user where t_with_user.user = 'foo';
1889+
----
1890+
3 foo
1891+
1892+
query T
1893+
select user from t_with_user;
1894+
----
1895+
test
1896+
NULL
1897+
foo
1898+
1899+
query IT
1900+
select * from t_with_user where user = 'foo';
1901+
----
1902+
3 foo
1903+
1904+
# test "current_time" column
1905+
# See https://github.com/apache/datafusion/issues/14141
1906+
statement count 0
1907+
create table t_with_current_time(a int, current_time text) as values (1,'now'), (2,null), (3,'later');
1908+
1909+
# here it's clear the the column was meant
1910+
query B
1911+
select t_with_current_time.current_time is not null from t_with_current_time;
1912+
----
1913+
true
1914+
false
1915+
true
1916+
1917+
# here it's the function
1918+
query B
1919+
select current_time is not null from t_with_current_time;
1920+
----
1921+
true
1922+
true
1923+
true
1924+
1925+
# and here it's the column again
1926+
query B
1927+
select "current_time" is not null from t_with_current_time;
1928+
----
1929+
true
1930+
false
1931+
true

0 commit comments

Comments
 (0)