Copilot commented on code in PR #22959:
URL: https://github.com/apache/datafusion/pull/22959#discussion_r3415370992
##########
datafusion/spark/src/function/string/concat_ws.rs:
##########
@@ -123,6 +124,30 @@ impl ScalarUDFImpl for SparkConcatWs {
spark_concat_ws(&args.args, args.number_rows)
}
+
+ fn simplify(
+ &self,
+ args: Vec<Expr>,
+ _info: &SimplifyContext,
+ ) -> Result<ExprSimplifyResult> {
+ if let Some(Expr::Literal(scalar, _)) = args.first()
+ && scalar.is_null()
+ {
+ return Ok(ExprSimplifyResult::Simplified(Expr::Literal(
+ ScalarValue::Utf8(None),
+ None,
+ )));
+ }
Review Comment:
The NULL-separator folding only triggers when the first argument is exactly
`Expr::Literal(..)`; depending on planner/optimizer ordering, `CAST(NULL AS
STRING)` can be represented as a cast/wrapper expression (e.g., `Expr::Cast`
around a NULL literal), which would bypass this rule and prevent the intended
simplification. Consider matching/peeling common wrappers (Cast/Alias/TryCast)
to reliably detect a typed NULL separator.
##########
datafusion/spark/src/function/string/concat_ws.rs:
##########
@@ -123,6 +124,30 @@ impl ScalarUDFImpl for SparkConcatWs {
spark_concat_ws(&args.args, args.number_rows)
}
+
+ fn simplify(
+ &self,
+ args: Vec<Expr>,
+ _info: &SimplifyContext,
+ ) -> Result<ExprSimplifyResult> {
+ if let Some(Expr::Literal(scalar, _)) = args.first()
+ && scalar.is_null()
+ {
+ return Ok(ExprSimplifyResult::Simplified(Expr::Literal(
+ ScalarValue::Utf8(None),
+ None,
+ )));
+ }
+
+ if matches!(args.as_slice(), [Expr::Literal(_, _)]) {
+ return Ok(ExprSimplifyResult::Simplified(Expr::Literal(
+ ScalarValue::Utf8(Some(String::new())),
+ None,
+ )));
+ }
Review Comment:
Both simplifications construct `Expr::Literal(.., None)` even though the
result type is known to be `Utf8`. Providing the explicit
`Some(DataType::Utf8)` here can reduce downstream inference/coercion ambiguity
and make the simplified expression's type stable (especially important inside
larger expressions).
--
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]
