quwin opened a new issue, #25038:
URL: https://github.com/apache/datafusion/issues/25038
### Describe the bug
A `CREATE FUNCTION` with an SQL body is accepted even when the `RETURN`
expression references a
positional placeholder beyond the declared arguments, or references any
argument when none were
declared. The invalid definition gets registered, and the error only appears
when the function
is first invoked.
For example, `better_add` declares two arguments but its body references
`$3`:
```sql
CREATE FUNCTION better_add(DOUBLE, DOUBLE)
RETURNS DOUBLE
RETURN $1 + $3
```
The `CREATE FUNCTION` succeeds there. The first call fails inside
optimization:
```
Optimizer rule 'simplify_expressions' failed
caused by
Execution error: Invalid placeholder, out of range: $3
```
A definition error like this should be reported at `CREATE FUNCTION` time
(plan error), not
deferred to every invocation. This gap is self-acknowledged by two in-code
FIXMEs introduced with
the named-variables/defaults work in #18450:
- `datafusion/sql/src/expr/value.rs` — "FIXME: In the CREATE FUNCTION
branch, param_type = None should raise an error"
- `datafusion/core/tests/user_defined/user_defined_scalar_functions.rs` —
"FIXME: Definitions with invalid placeholders are allowed, fail at runtime"
### To Reproduce
SQL-function DDL requires a configured `FunctionFactory` (a plain
`SessionContext` returns
"Function factory has not been configured"), so the repro uses the shipped
`function_factory` example. In
`datafusion-examples/examples/builtin_functions/function_factory.rs`,
add a third function alongside `f1`/`f2`:
```rust
// f3 is declared with two arguments, but its body references $3
let sql = r#"
CREATE FUNCTION f3(BIGINT, BIGINT)
RETURNS BIGINT
RETURN $1 + $3
"#;
ctx.sql(sql).await?.show().await?;
ctx.sql("SELECT f3(1, 2)").await?.show().await?;
```
then run:
```bash
cargo run --example builtin_functions -- function_factory
```
The same shape is covered by the existing integration test
`create_scalar_function_from_sql_statement` in
`datafusion/core/tests/user_defined/user_defined_scalar_functions.rs`.
### Expected behavior
`CREATE FUNCTION better_add(DOUBLE, DOUBLE) ... RETURN $1 + $3` should fail
during planning with
an error such as:
```
Error during planning: Invalid placeholder, out of range: $3
```
(PostgreSQL analog: `ERROR: there is no parameter $3`.) Named placeholders
with zero declared
arguments (e.g. `CREATE FUNCTION f() RETURNS DOUBLE RETURN $a`) should
likewise be rejected at
definition time. Valid positional, named, and defaulted-argument bodies that
only
reference declared arguments must keep working.
### Additional context
- Verified on `main` at `e1ca94f`
- Root cause: the `RETURN` body is planned with PREPARE-style parameter types
(`create_placeholder_expr` in `datafusion/sql/src/expr/value.rs`); an
out-of-range `$N` gets an
untyped `Placeholder` instead of an error because that code path is shared
with `PREPARE`, where
an empty/unknown parameter list must stay permissive for deferred type
inference. The place
where both the declared argument list and the parsed body are available is
the
`Statement::CreateFunction` arm of the SQL planner, so a validation pass
there (walking the body
for `Expr::Placeholder`) would fix every `FunctionFactory` implementation
with zero impact on
`PREPARE` semantics. Runtime guards in factories would remain as a
defensive backstop.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
