EeshanBembi commented on code in PR #18137:
URL: https://github.com/apache/datafusion/pull/18137#discussion_r2658922142
##########
datafusion/functions/src/string/concat.rs:
##########
@@ -302,59 +421,85 @@ pub(crate) fn simplify_concat(args: Vec<Expr>) ->
Result<ExprSimplifyResult> {
ConcatFunc::new().return_type(&data_types)
}?;
- for arg in args.clone() {
+ for arg in args.iter() {
match arg {
Expr::Literal(ScalarValue::Utf8(None), _) => {}
- Expr::Literal(ScalarValue::LargeUtf8(None), _) => {
- }
- Expr::Literal(ScalarValue::Utf8View(None), _) => { }
+ Expr::Literal(ScalarValue::LargeUtf8(None), _) => {}
+ Expr::Literal(ScalarValue::Utf8View(None), _) => {}
// filter out `null` args
// All literals have been converted to Utf8 or LargeUtf8 in
type_coercion.
// Concatenate it with the `contiguous_scalar`.
Expr::Literal(ScalarValue::Utf8(Some(v)), _) => {
- contiguous_scalar += &v;
+ contiguous_scalar += v;
}
Expr::Literal(ScalarValue::LargeUtf8(Some(v)), _) => {
- contiguous_scalar += &v;
+ contiguous_scalar += v;
}
Expr::Literal(ScalarValue::Utf8View(Some(v)), _) => {
- contiguous_scalar += &v;
+ contiguous_scalar += v;
}
- Expr::Literal(x, _) => {
- return internal_err!(
- "The scalar {x} should be casted to string type during the
type coercion."
- )
+ Expr::Literal(scalar_val, _) => {
+ // Convert non-string, non-array literals to their string
representation
+ // Skip array literals - they should be handled at runtime
Review Comment:
The literal handling is needed because simplification happens before
coercion. I've added a comment to clarify:
` // This is needed during simplification phase which happens before
coercion`
This ensures numeric literals like concat('hello', 42) work correctly.
--
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]
