feat: support named variables & defaults for CREATE FUNCTION

[r1b]

Which issue does this PR close?

• Closes It would be great if we could use variable names instead of $1 and support default values in CREATE FUNCTION #17887.

Rationale for this change

See linked issue above

What changes are included in this PR?

• Add validation when planning CreateFunction
  • Enforce consistent parameter style (positional or named)
  • Non-default params cannot follow default params
• CreateFunction parameter names are now preserved from the parse tree
• If we encounter a named parameter when constructing a Placeholder
  • We try to rewrite this to a positional parameter from the available param types
  • If no matching param type is found, report an error
• Update ScalarFunctionWrapper to handle defaults
  • Preserve the parsed defaults
  • Generate all valid signatures for all possible combinations of arguments
  • Fall back to default expr when no matching argument is provided

Are these changes tested?

Yes, see added / adjusted unit tests

Are there any user-facing changes?

Yes, if the approach here is acceptable we should update the SQL UDF examples in datafusion-examples/examples/function_factory.rs (TODO).

Also, note that due to ambiguity between PREPARE and CREATE FUNCTION param context one error message now has reduced fidelity.

Invalid placeholder, not a number: $foo

This can now be triggered either by using named params in a prepared statement or when referencing an undefined named param in a SQL UDF. New message:

Unknown placeholder: $foo

Finally, there are two additional user-facing errors when planning CreateFunction:

• When named / positional parameter styles are mixed
• When non-default arguments follow default arguments

[r1b]

note I'm not sure this is actually necessary, but it made it easier to reason about the changes. If we think this is valuable I can look at relaxing this constraint.

[r1b]

note I explored doing this without rewriting to positional params, but I couldn't see a path forward without either:

• Adding a way to distinguish between prepared statement vs SQL UDF context
• Supporting named params in prepared statements (not currently supported in sqlparser AFAICT)

[r1b]

Test failure seems like a flake:

<...>
1. query result mismatch:
[SQL] SELECT shuffle(['a', 'b', 'c', 'd', 'e', 'f']) != ['a', 'b', 'c', 'd', 'e', 'f'];;
[Diff] (-expected|+actual)
-   true
+   false
at /__w/datafusion/datafusion/datafusion/sqllogictest/test_files/spark/array/shuffle.slt:36
<...>

[r1b]

Thanks for the feedback @Jefffrey, updated

[martin-g]

Please add a test case with positional parameter that is not existing, e.g. $5 where there are less than 5 arguments.
I have the feeling that it will fail with index out of bounds error at https://github.com/apache/datafusion/pull/18450/files#diff-647d2e08b4d044bf63b35f9e23092ba9673b80b1568e8f3abffd7f909552ea1aR999

You need to add a check similar to if placeholder_position < defaults.len() {...} around it and return an error in the else clause

[r1b]

Thanks, addressed in 0330adb.

This case also revealed that the DEFAULT syntax is broken for positional params. I switched to = syntax and added a test that illustrates the problem in a97ddb5.

Ref: https://github.com/apache/datafusion-sqlparser-rs/blob/308a7231bcbc5c1c8ab71fe38f17b1a21632a6c6/src/parser/mod.rs#L5536

EDIT: It seems that = is the "canonical" syntax in any case

[r1b]

Proposed a fix for the DEFAULT syntax bug upstream: apache/datafusion-sqlparser-rs#2091