namanjain24-sudo commented on code in PR #25442:
URL: https://github.com/apache/datafusion/pull/25442#discussion_r4195910887


##########
datafusion/substrait/src/logical_plan/consumer/expr/scalar_function.rs:
##########
@@ -187,6 +187,32 @@ fn arg_list_to_binary_op_tree_inner(
     }))
 }
 
+/// Reads the `case_sensitivity` option of a `like` call.
+///
+/// Substrait says a consumer must use the first value it supports, and must
+/// reject the call when it supports none of them.
+fn case_insensitive_option(f: &ScalarFunction) -> Result<bool> {
+    let Some(option) = f
+        .options
+        .iter()
+        .find(|option| option.name.eq_ignore_ascii_case("case_sensitivity"))
+    else {
+        return Ok(false);
+    };
+    for preference in &option.preference {
+        if preference.eq_ignore_ascii_case("CASE_SENSITIVE") {

Review Comment:
   Added case_insensitive_option_cases: covers preference ordering (including 
CASE_INSENSITIVE_ASCII being skipped since it's unsupported), mixed-case 
option/preference names, an empty preference list, and full rejection.



##########
datafusion/substrait/src/logical_plan/producer/expr/scalar_function.rs:
##########
@@ -246,35 +251,65 @@ pub fn from_like(
         escape_char,
         case_insensitive,
     } = like;
-    let function_anchor = if *case_insensitive {
-        producer.register_function("ilike".to_string())
-    } else {
-        producer.register_function("like".to_string())
-    };
     // Substrait documents `output_type` as "Must be set to the return type of
     // the function, exactly as derived using the declaration in the 
extension",
     // and a consumer that reads it rejects the call when it is unset. The type
     // comes from the expression itself so that it matches what DataFusion
     // derives, rather than being restated here.
     let (_, output_field) = Expr::Like(like.clone()).to_field(schema)?;
     let output_type = to_substrait_type_from_field(producer, &output_field)?;
+
+    make_substrait_like_expr(
+        producer,
+        *case_insensitive,
+        *negated,
+        expr,
+        pattern,
+        *escape_char,
+        schema,
+        output_type,
+    )
+}
+
+#[expect(clippy::too_many_arguments)]
+fn make_substrait_like_expr(
+    producer: &mut impl SubstraitProducer,
+    ignore_case: bool,
+    negated: bool,

Review Comment:
   Done — inlined it back into from_like (single caller), so the 
too_many_arguments suppression is gone.



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