Blizzara commented on code in PR #11234: URL: https://github.com/apache/datafusion/pull/11234#discussion_r1664841405
########## datafusion/substrait/src/logical_plan/consumer.rs: ########## @@ -2007,14 +2051,21 @@ impl BuiltinExprBuilder { .await? .as_ref() .clone(); - let Some(ArgType::Value(escape_char_substrait)) = &f.arguments[2].arg_type else { - return substrait_err!("Invalid arguments type for `{fn_name}` expr"); - }; - let escape_char_expr = + + // Default case: escape character is Literal(Utf8(None)) + let escape_char_expr = if f.arguments.len() == 3 { + let Some(ArgType::Value(escape_char_substrait)) = &f.arguments[2].arg_type + else { + return substrait_err!("Invalid arguments type for `{fn_name}` expr"); + }; from_substrait_rex(ctx, escape_char_substrait, input_schema, extensions) .await? .as_ref() - .clone(); + .clone() + } else { + Expr::Literal(ScalarValue::Utf8(None)) Review Comment: this just gets unwrapped on the next lines, maybe just move the unwrapping into the if clause above and then return None here directly? -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org