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

Reply via email to