Add Expr->String for ScalarFunction and InList #9759
Conversation
| args: vec![col("a"), col("b")], | ||
| }), | ||
| r#"dummy_udf("a", "b")"#, | ||
| ), |
There was a problem hiding this comment.
I think for ScalarFunction, it's better to add test cases for UDF and BuiltIn.
But since the function port work is going to be finished, I doubt whether it's necessary to do that. :)
There was a problem hiding this comment.
Since there is no code specific to BuiltInScalarFunction in the unparser code, I agree there is no reason to add a specific test here
datafusion/sql/src/unparser/expr.rs
Outdated
| @@ -561,6 +636,31 @@ mod tests { | |||
| }), | |||
| r#"CAST("a" AS INTEGER UNSIGNED)"#, | |||
| ), | |||
| ( | |||
| Expr::InList(InList { | |||
There was a problem hiding this comment.
I think you can write this more concisely with the expr API -- https://docs.rs/datafusion/latest/datafusion/logical_expr/enum.Expr.html#method.in_list
Something like
col("a").in(vec![lit(1), lit(2), lit(3)])Likewise for NOT IN
col("a").not_in(vec![lit(1), lit(2), lit(3)])| args: vec![col("a"), col("b")], | ||
| }), | ||
| r#"dummy_udf("a", "b")"#, | ||
| ), |
There was a problem hiding this comment.
Since there is no code specific to BuiltInScalarFunction in the unparser code, I agree there is no reason to add a specific test here
datafusion/sql/src/unparser/expr.rs
Outdated
| Expr::ScalarFunction(ScalarFunction { | ||
| func_def: ScalarFunctionDefinition::UDF(Arc::new( | ||
| ScalarUDF::new_from_impl(DummyUDF::new()), | ||
| )), | ||
| args: vec![col("a"), col("b")], | ||
| }), |
There was a problem hiding this comment.
How about using the call API https://docs.rs/datafusion/latest/datafusion/logical_expr/struct.ScalarUDF.html#method.call
ScalarUDF::new_from_impl(DummyUDF::new())
.call(vec![col("a"), col("b")])There was a problem hiding this comment.
Love this, more concise!
Which issue does this PR close?
Related #9726.
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?