Blizzara commented on code in PR #11234:
URL: https://github.com/apache/datafusion/pull/11234#discussion_r1665820394


##########
datafusion/substrait/src/logical_plan/consumer.rs:
##########
@@ -2007,26 +2038,33 @@ 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();
-        let Expr::Literal(ScalarValue::Utf8(escape_char)) = escape_char_expr 
else {
-            return substrait_err!(
-                "Expect Utf8 literal for escape char, but found 
{escape_char_expr:?}"
-            );
+                .clone()
+        } else {
+            Expr::Literal(ScalarValue::Utf8(None))
         };
 
-        Ok(Arc::new(Expr::Like(Like {
-            negated: false,
-            expr: Box::new(expr),
-            pattern: Box::new(pattern),
-            escape_char: escape_char.map(|c| c.chars().next().unwrap()),
-            case_insensitive,
-        })))
+        if let Expr::Literal(ScalarValue::Utf8(escape_char)) = 
escape_char_expr {
+            Ok(Arc::new(Expr::Like(Like {
+                negated: false,
+                expr: Box::new(expr),
+                pattern: Box::new(pattern),
+                escape_char: escape_char.map(|c| c.chars().next().unwrap()),
+                case_insensitive,
+            })))
+        } else {
+            substrait_err!(
+                "Expect Utf8 literal for escape char, but found 
{escape_char_expr:?}"
+            )

Review Comment:
   this wasn't exactly what I had in mind 😅 , I meant more like:
   ```
           let escape_char = 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");
               };
               let escape_char_expr = from_substrait_rex(ctx, 
escape_char_substrait, input_schema, extensions)
                   .await?
                   .as_ref()
                   .clone();
              escape_char_expr match {
                  Expr::Literal(ScalarValue::Utf8(escape_char)) => escape_char,
                  _ => return substrait_err!("Expect Utf8 literal for escape 
char, but found {escape_char_expr:?}")
              }
           } else {
               None
           };
   
           Ok(Arc::new(Expr::Like(Like {
               negated: false,
               expr: Box::new(expr),
               pattern: Box::new(pattern),
               escape_char,
               case_insensitive,
           })))
   ```



-- 
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]

Reply via email to