CREATE FUNCTION accepts placeholders that don't match a declared argument; the error is deferred to call time
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### 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.
Contributor guide
Assessment
This issue has not been assessed yet.