alamb commented on code in PR #16456:
URL: https://github.com/apache/datafusion/pull/16456#discussion_r2283002596


##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -823,13 +826,12 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
             return not_impl_err!("ANY in LIKE expression");
         }
         let pattern = self.sql_expr_to_logical_expr(pattern, schema, 
planner_context)?;
-        let escape_char = if let Some(char) = escape_char {
-            if char.len() != 1 {
-                return plan_err!("Invalid escape character in LIKE 
expression");
+        let escape_char = match escape_char {
+            Some(Value::SingleQuotedString(char)) if char.len() == 1 => {
+                Some(char.chars().next().unwrap())
             }
-            Some(char.chars().next().unwrap())
-        } else {
-            None
+            Some(_) => return plan_err!("Invalid escape character in LIKE 
expression"),

Review Comment:
   It is nothing this PR did, but these messages would be nicer if they told 
the user what was supported
   
   Maybe something like
   
   ```suggestion
               Some(_) => return plan_err!("Invalid escape character 
'{escape_char}' in LIKE expression. Expected a single character"),
   ```
   
   The same comment applies to the SIMILAR TO syntax below



##########
datafusion/sql/src/expr/substring.rs:
##########
@@ -77,8 +78,16 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
             }
         }
 
-        not_impl_err!(
-            "Substring not supported by UserDefinedExtensionPlanners: 
{substring_args:?}"
-        )
+        let fun = self

Review Comment:
   This basically hard codes the use of a function named "substr" into the 
planner
   
   I think the preferred way is to use the `ExprPlanner` API, similarly to how 
plan field access is done here:
   
   
https://github.com/apache/datafusion/blob/d987d2dc80c702c86d7ac853fb9c251a291d7b91/datafusion/expr/src/planner.rs#L134



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