Skip to content

Commit b04cc1d

Browse files
fix: Emit LIKE the way the Substrait extension defines it
The extension defines one function, `like`, taking two arguments and carrying case sensitivity as the `case_sensitivity` option. The producer emitted three arguments, registered `ilike` for the case insensitive form, which no extension defines, and never set the option. substrait-java resolves `ilike` to nothing, and Spark rejects the three argument call with WRONG_NUM_ARGS. The producer now registers `like`, emits the two arguments, and sets the option for ILIKE. An escape character has no place in that definition, so LIKE ... ESCAPE is rejected rather than emitted as a third argument. The consumer reads the option, taking the first value it supports, and still accepts the `ilike` name and the three argument form so that plans from an older DataFusion keep loading.
1 parent e5fdc9d commit b04cc1d

2 files changed

Lines changed: 162 additions & 18 deletions

File tree

‎datafusion/substrait/src/logical_plan/consumer/expr/scalar_function.rs‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,32 @@ fn arg_list_to_binary_op_tree_inner(
187187
}))
188188
}
189189

190+
/// Reads the `case_sensitivity` option of a `like` call.
191+
///
192+
/// Substrait says a consumer must use the first value it supports, and must
193+
/// reject the call when it supports none of them.
194+
fn case_insensitive_option(f: &ScalarFunction) -> Result<bool> {
195+
let Some(option) = f
196+
.options
197+
.iter()
198+
.find(|option| option.name.eq_ignore_ascii_case("case_sensitivity"))
199+
else {
200+
return Ok(false);
201+
};
202+
for preference in &option.preference {
203+
if preference.eq_ignore_ascii_case("CASE_SENSITIVE") {
204+
return Ok(false);
205+
}
206+
if preference.eq_ignore_ascii_case("CASE_INSENSITIVE") {
207+
return Ok(true);
208+
}
209+
}
210+
not_impl_err!(
211+
"Unsupported case_sensitivity for `like`: {:?}",
212+
option.preference
213+
)
214+
}
215+
190216
/// Build [`Expr`] from its name and required inputs.
191217
struct BuiltinExprBuilder {
192218
expr_name: String,
@@ -213,7 +239,10 @@ impl BuiltinExprBuilder {
213239
args: Vec<Expr>,
214240
) -> Result<Expr> {
215241
match self.expr_name.as_str() {
216-
"like" => Self::build_like_expr(false, false, f, args),
242+
// `like` carries case sensitivity as an option. `ilike` is not a
243+
// Substrait function, but DataFusion used to emit it, so plans
244+
// written by an older version are still read.
245+
"like" => Self::build_like_expr(case_insensitive_option(f)?, false, f, args),
217246
"ilike" => Self::build_like_expr(true, false, f, args),
218247
"like_match" => Self::build_like_expr(false, false, f, args),
219248
"like_imatch" => Self::build_like_expr(true, false, f, args),

‎datafusion/substrait/src/logical_plan/producer/expr/scalar_function.rs‎

Lines changed: 132 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,17 @@
1616
// under the License.
1717

1818
use crate::logical_plan::producer::{
19-
SubstraitProducer, to_substrait_literal_expr, to_substrait_type,
20-
to_substrait_type_from_field,
19+
SubstraitProducer, to_substrait_type, to_substrait_type_from_field,
2120
};
2221
use datafusion::arrow::datatypes::DataType;
2322
use datafusion::common::datatype::FieldExt;
2423
use datafusion::common::{
25-
DFSchemaRef, ScalarValue, internal_datafusion_err, not_impl_err, substrait_err,
24+
DFSchemaRef, internal_datafusion_err, not_impl_err, substrait_err,
2625
};
2726
use datafusion::logical_expr::{
2827
Between, BinaryExpr, Expr, ExprSchemable, Like, Operator, expr,
2928
};
29+
use substrait::proto::FunctionOption;
3030
use substrait::proto::expression::{RexType, ScalarFunction};
3131
use substrait::proto::function_argument::ArgType;
3232
use substrait::proto::{Expression, FunctionArgument, Type};
@@ -234,6 +234,11 @@ pub fn from_binary_expr(
234234
))
235235
}
236236

237+
/// The option `like` uses to carry case sensitivity, and the value that asks
238+
/// for the case insensitive form. Defined in `functions_string.yaml`.
239+
pub(crate) const CASE_SENSITIVITY_OPTION: &str = "case_sensitivity";
240+
pub(crate) const CASE_INSENSITIVE: &str = "CASE_INSENSITIVE";
241+
237242
pub fn from_like(
238243
producer: &mut impl SubstraitProducer,
239244
like: &Like,
@@ -246,35 +251,65 @@ pub fn from_like(
246251
escape_char,
247252
case_insensitive,
248253
} = like;
249-
let function_anchor = if *case_insensitive {
250-
producer.register_function("ilike".to_string())
251-
} else {
252-
producer.register_function("like".to_string())
253-
};
254254
// Substrait documents `output_type` as "Must be set to the return type of
255255
// the function, exactly as derived using the declaration in the extension",
256256
// and a consumer that reads it rejects the call when it is unset. The type
257257
// comes from the expression itself so that it matches what DataFusion
258258
// derives, rather than being restated here.
259259
let (_, output_field) = Expr::Like(like.clone()).to_field(schema)?;
260260
let output_type = to_substrait_type_from_field(producer, &output_field)?;
261+
262+
make_substrait_like_expr(
263+
producer,
264+
*case_insensitive,
265+
*negated,
266+
expr,
267+
pattern,
268+
*escape_char,
269+
schema,
270+
output_type,
271+
)
272+
}
273+
274+
#[expect(clippy::too_many_arguments)]
275+
fn make_substrait_like_expr(
276+
producer: &mut impl SubstraitProducer,
277+
ignore_case: bool,
278+
negated: bool,
279+
expr: &Expr,
280+
pattern: &Expr,
281+
escape_char: Option<char>,
282+
schema: &DFSchemaRef,
283+
output_type: Type,
284+
) -> datafusion::common::Result<Expression> {
285+
// `like` takes two arguments and carries case sensitivity as an option;
286+
// the extensions define no `ilike` and no escape character, so an escape
287+
// has no encoding here and is rejected rather than emitted as a third
288+
// argument that a consumer would bind to a parameter the function does
289+
// not have.
290+
if escape_char.is_some() {
291+
return not_impl_err!("Substrait does not define an escape character for `like`");
292+
}
293+
let function_anchor = producer.register_function("like".to_string());
261294
let expr = producer.handle_expr(expr, schema)?;
262295
let pattern = producer.handle_expr(pattern, schema)?;
263-
let escape_char = to_substrait_literal_expr(
264-
producer,
265-
&ScalarValue::Utf8(escape_char.map(|c| c.to_string())),
266-
)?;
267296
let arguments = vec![
268297
FunctionArgument {
269298
arg_type: Some(ArgType::Value(expr)),
270299
},
271300
FunctionArgument {
272301
arg_type: Some(ArgType::Value(pattern)),
273302
},
274-
FunctionArgument {
275-
arg_type: Some(ArgType::Value(escape_char)),
276-
},
277303
];
304+
// An unset option leaves the default, which is `CASE_SENSITIVE`.
305+
let options = if ignore_case {
306+
vec![FunctionOption {
307+
name: CASE_SENSITIVITY_OPTION.to_string(),
308+
preference: vec![CASE_INSENSITIVE.to_string()],
309+
}]
310+
} else {
311+
vec![]
312+
};
278313

279314
#[expect(deprecated)]
280315
let substrait_like = Expression {
@@ -283,11 +318,11 @@ pub fn from_like(
283318
arguments,
284319
output_type: Some(output_type.clone()),
285320
args: vec![],
286-
options: vec![],
321+
options,
287322
})),
288323
};
289324

290-
if *negated {
325+
if negated {
291326
let function_anchor = producer.register_function("not".to_string());
292327

293328
#[expect(deprecated)]
@@ -438,6 +473,7 @@ pub fn operator_to_name(op: Operator) -> &'static str {
438473

439474
#[cfg(test)]
440475
mod tests {
476+
use super::{CASE_INSENSITIVE, CASE_SENSITIVITY_OPTION};
441477
use crate::logical_plan::producer::{
442478
DefaultSubstraitProducer, SubstraitProducer, to_substrait_type,
443479
};
@@ -447,9 +483,88 @@ mod tests {
447483
use datafusion::logical_expr::{Expr, Like};
448484
use datafusion::prelude::{col, lit};
449485
use substrait::proto::Expression;
486+
use substrait::proto::FunctionOption;
450487
use substrait::proto::expression::{RexType, ScalarFunction};
451488
use substrait::proto::function_argument::ArgType;
452489

490+
/// `like` takes two arguments and carries case sensitivity as an option,
491+
/// so `ILIKE` is that option rather than a separate function.
492+
#[tokio::test]
493+
async fn like_emits_case_sensitivity_option() -> datafusion::common::Result<()> {
494+
let state = SessionStateBuilder::default().build();
495+
let schema =
496+
DFSchemaRef::new(DFSchema::try_from(Schema::new(vec![Field::new(
497+
"s",
498+
DataType::Utf8,
499+
true,
500+
)]))?);
501+
502+
for (case_insensitive, expected_options) in [
503+
(false, vec![]),
504+
(
505+
true,
506+
vec![FunctionOption {
507+
name: CASE_SENSITIVITY_OPTION.to_string(),
508+
preference: vec![CASE_INSENSITIVE.to_string()],
509+
}],
510+
),
511+
] {
512+
let mut producer = DefaultSubstraitProducer::new(&state);
513+
let like = Like::new(
514+
false,
515+
Box::new(col("s")),
516+
Box::new(lit("a%")),
517+
None,
518+
case_insensitive,
519+
);
520+
let expr = producer.handle_expr(&Expr::Like(like), &schema)?;
521+
522+
let Some(RexType::ScalarFunction(call)) = expr.rex_type else {
523+
panic!("Substrait ScalarFunction expected")
524+
};
525+
assert_eq!(
526+
producer
527+
.get_extensions()
528+
.functions
529+
.get(&call.function_reference),
530+
Some(&"like".to_string()),
531+
"case_insensitive = {case_insensitive}"
532+
);
533+
assert_eq!(call.arguments.len(), 2, "no escape argument is emitted");
534+
assert_eq!(call.options, expected_options);
535+
}
536+
537+
Ok(())
538+
}
539+
540+
/// The extensions define no escape character, so there is nothing to emit.
541+
#[tokio::test]
542+
async fn like_with_escape_is_rejected() -> datafusion::common::Result<()> {
543+
let state = SessionStateBuilder::default().build();
544+
let schema =
545+
DFSchemaRef::new(DFSchema::try_from(Schema::new(vec![Field::new(
546+
"s",
547+
DataType::Utf8,
548+
true,
549+
)]))?);
550+
let mut producer = DefaultSubstraitProducer::new(&state);
551+
552+
let like = Like::new(
553+
false,
554+
Box::new(col("s")),
555+
Box::new(lit("a!%")),
556+
Some('!'),
557+
false,
558+
);
559+
let err = producer
560+
.handle_expr(&Expr::Like(like), &schema)
561+
.expect_err("an escape character must be rejected")
562+
.to_string();
563+
assert!(err.contains("escape character"), "unexpected error: {err}");
564+
565+
Ok(())
566+
}
567+
453568
#[tokio::test]
454569
async fn binary_expr_output_type() -> datafusion::common::Result<()> {
455570
let state = SessionStateBuilder::default().build();

0 commit comments

Comments
 (0)