From 1ed19045050fe40cc6ee07d096f87cb9e2fc08fb Mon Sep 17 00:00:00 2001 From: Evgenii Khramkov Date: Wed, 23 Apr 2025 10:34:55 +0900 Subject: [PATCH 1/3] Fix ILIKE expression support in SQL unparser --- datafusion/sql/src/unparser/expr.rs | 81 ++++++++++++++++++----- datafusion/sql/tests/cases/plan_to_sql.rs | 27 ++++++++ 2 files changed, 93 insertions(+), 15 deletions(-) diff --git a/datafusion/sql/src/unparser/expr.rs b/datafusion/sql/src/unparser/expr.rs index 1826ef340faf..4af4de4b229c 100644 --- a/datafusion/sql/src/unparser/expr.rs +++ b/datafusion/sql/src/unparser/expr.rs @@ -261,26 +261,57 @@ impl Unparser<'_> { uses_odbc_syntax: false, })) } - Expr::SimilarTo(Like { + Expr::Like(Like { negated, expr, pattern, escape_char, - case_insensitive: _, - }) - | Expr::Like(Like { + case_insensitive, + }) => { + if *case_insensitive { + Ok(ast::Expr::ILike { + negated: *negated, + expr: Box::new(self.expr_to_sql_inner(expr)?), + pattern: Box::new(self.expr_to_sql_inner(pattern)?), + escape_char: escape_char.map(|c| c.to_string()), + any: false, + }) + } else { + Ok(ast::Expr::Like { + negated: *negated, + expr: Box::new(self.expr_to_sql_inner(expr)?), + pattern: Box::new(self.expr_to_sql_inner(pattern)?), + escape_char: escape_char.map(|c| c.to_string()), + any: false, + }) + } + } + Expr::SimilarTo(Like { negated, expr, pattern, escape_char, - case_insensitive: _, - }) => Ok(ast::Expr::Like { - negated: *negated, - expr: Box::new(self.expr_to_sql_inner(expr)?), - pattern: Box::new(self.expr_to_sql_inner(pattern)?), - escape_char: escape_char.map(|c| c.to_string()), - any: false, - }), + case_insensitive, + }) => { + if *case_insensitive { + Ok(ast::Expr::ILike { + negated: *negated, + expr: Box::new(self.expr_to_sql_inner(expr)?), + pattern: Box::new(self.expr_to_sql_inner(pattern)?), + escape_char: escape_char.map(|c| c.to_string()), + any: false, + }) + } else { + Ok(ast::Expr::Like { + negated: *negated, + expr: Box::new(self.expr_to_sql_inner(expr)?), + pattern: Box::new(self.expr_to_sql_inner(pattern)?), + escape_char: escape_char.map(|c| c.to_string()), + any: false, + }) + } + } + Expr::AggregateFunction(agg) => { let func_name = agg.func.name(); @@ -970,7 +1001,7 @@ impl Unparser<'_> { DataType::Timestamp(unit, _) => unit, _ => return Err(internal_datafusion_err!("Expected Timestamp, got {:?}", T::DATA_TYPE)), }; - + Ok(ast::Expr::Cast { kind: ast::CastKind::Cast, expr: Box::new(ast::Expr::Value(SingleQuotedString(ts))), @@ -1797,7 +1828,7 @@ mod tests { expr: Box::new(col("a")), pattern: Box::new(lit("foo")), escape_char: Some('o'), - case_insensitive: true, + case_insensitive: false, }), r#"a NOT LIKE 'foo' ESCAPE 'o'"#, ), @@ -1807,10 +1838,30 @@ mod tests { expr: Box::new(col("a")), pattern: Box::new(lit("foo")), escape_char: Some('o'), - case_insensitive: true, + case_insensitive: false, }), r#"a LIKE 'foo' ESCAPE 'o'"#, ), + ( + Expr::Like(Like { + negated: true, + expr: Box::new(col("a")), + pattern: Box::new(lit("foo")), + escape_char: Some('o'), + case_insensitive: true, + }), + r#"a NOT ILIKE 'foo' ESCAPE 'o'"#, + ), + ( + Expr::SimilarTo(Like { + negated: false, + expr: Box::new(col("a")), + pattern: Box::new(lit("foo")), + escape_char: Some('o'), + case_insensitive: true, + }), + r#"a ILIKE 'foo' ESCAPE 'o'"#, + ), ( Expr::Literal(ScalarValue::Date64(Some(0))), r#"CAST('1970-01-01 00:00:00' AS DATETIME)"#, diff --git a/datafusion/sql/tests/cases/plan_to_sql.rs b/datafusion/sql/tests/cases/plan_to_sql.rs index c7da91df50f3..ffb07508b26b 100644 --- a/datafusion/sql/tests/cases/plan_to_sql.rs +++ b/datafusion/sql/tests/cases/plan_to_sql.rs @@ -1533,6 +1533,33 @@ fn test_unnest_to_sql() { ); } +#[test] +fn test_like_filters() { + sql_round_trip( + GenericDialect {}, + r#"SELECT * FROM person WHERE first_name LIKE '%John%'"#, + r#"SELECT * FROM person WHERE person.first_name LIKE '%John%'"#, + ); + + sql_round_trip( + GenericDialect {}, + r#"SELECT * FROM person WHERE first_name ILIKE '%john%'"#, + r#"SELECT * FROM person WHERE person.first_name ILIKE '%john%'"#, + ); + + sql_round_trip( + GenericDialect {}, + r#"SELECT * FROM person WHERE first_name NOT LIKE 'A%'"#, + r#"SELECT * FROM person WHERE person.first_name NOT LIKE 'A%'"#, + ); + + sql_round_trip( + GenericDialect {}, + r#"SELECT * FROM person WHERE first_name NOT ILIKE 'a%'"#, + r#"SELECT * FROM person WHERE person.first_name NOT ILIKE 'a%'"#, + ); +} + #[test] fn test_join_with_no_conditions() { sql_round_trip( From c00431c12ea0a19fbc56d9755b5f7cc0f0c22e6f Mon Sep 17 00:00:00 2001 From: Evgenii Khramkov Date: Wed, 23 Apr 2025 12:18:11 +0900 Subject: [PATCH 2/3] fix pattern matching order and adjust tests --- datafusion/sql/src/unparser/expr.rs | 50 +++++++++++------------------ 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/datafusion/sql/src/unparser/expr.rs b/datafusion/sql/src/unparser/expr.rs index 4af4de4b229c..4c4aeb472b9c 100644 --- a/datafusion/sql/src/unparser/expr.rs +++ b/datafusion/sql/src/unparser/expr.rs @@ -261,32 +261,20 @@ impl Unparser<'_> { uses_odbc_syntax: false, })) } - Expr::Like(Like { + Expr::SimilarTo(Like { negated, expr, pattern, escape_char, - case_insensitive, - }) => { - if *case_insensitive { - Ok(ast::Expr::ILike { - negated: *negated, - expr: Box::new(self.expr_to_sql_inner(expr)?), - pattern: Box::new(self.expr_to_sql_inner(pattern)?), - escape_char: escape_char.map(|c| c.to_string()), - any: false, - }) - } else { - Ok(ast::Expr::Like { - negated: *negated, - expr: Box::new(self.expr_to_sql_inner(expr)?), - pattern: Box::new(self.expr_to_sql_inner(pattern)?), - escape_char: escape_char.map(|c| c.to_string()), - any: false, - }) - } - } - Expr::SimilarTo(Like { + case_insensitive: _, + }) => Ok(ast::Expr::Like { + negated: *negated, + expr: Box::new(self.expr_to_sql_inner(expr)?), + pattern: Box::new(self.expr_to_sql_inner(pattern)?), + escape_char: escape_char.map(|c| c.to_string()), + any: false, + }), + Expr::Like(Like { negated, expr, pattern, @@ -1833,24 +1821,24 @@ mod tests { r#"a NOT LIKE 'foo' ESCAPE 'o'"#, ), ( - Expr::SimilarTo(Like { - negated: false, + Expr::Like(Like { + negated: true, expr: Box::new(col("a")), pattern: Box::new(lit("foo")), escape_char: Some('o'), - case_insensitive: false, + case_insensitive: true, }), - r#"a LIKE 'foo' ESCAPE 'o'"#, + r#"a NOT ILIKE 'foo' ESCAPE 'o'"#, ), ( - Expr::Like(Like { - negated: true, + Expr::SimilarTo(Like { + negated: false, expr: Box::new(col("a")), pattern: Box::new(lit("foo")), escape_char: Some('o'), - case_insensitive: true, + case_insensitive: false, }), - r#"a NOT ILIKE 'foo' ESCAPE 'o'"#, + r#"a LIKE 'foo' ESCAPE 'o'"#, ), ( Expr::SimilarTo(Like { @@ -1860,7 +1848,7 @@ mod tests { escape_char: Some('o'), case_insensitive: true, }), - r#"a ILIKE 'foo' ESCAPE 'o'"#, + r#"a LIKE 'foo' ESCAPE 'o'"#, ), ( Expr::Literal(ScalarValue::Date64(Some(0))), From 6c506f0729403aa6bf7794a09cdde5abe9ec9c8d Mon Sep 17 00:00:00 2001 From: Evgenii Khramkov Date: Wed, 23 Apr 2025 12:26:16 +0900 Subject: [PATCH 3/3] add LIKE/ILIKE + ESCAPE tests --- datafusion/sql/tests/cases/plan_to_sql.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/datafusion/sql/tests/cases/plan_to_sql.rs b/datafusion/sql/tests/cases/plan_to_sql.rs index ffb07508b26b..729c51dd643e 100644 --- a/datafusion/sql/tests/cases/plan_to_sql.rs +++ b/datafusion/sql/tests/cases/plan_to_sql.rs @@ -1558,6 +1558,24 @@ fn test_like_filters() { r#"SELECT * FROM person WHERE first_name NOT ILIKE 'a%'"#, r#"SELECT * FROM person WHERE person.first_name NOT ILIKE 'a%'"#, ); + + sql_round_trip( + GenericDialect {}, + r#"SELECT * FROM person WHERE first_name LIKE 'A!_%' ESCAPE '!'"#, + r#"SELECT * FROM person WHERE person.first_name LIKE 'A!_%' ESCAPE '!'"#, + ); + + sql_round_trip( + GenericDialect {}, + r#"SELECT * FROM person WHERE first_name NOT LIKE 'A!_%' ESCAPE '!'"#, + r#"SELECT * FROM person WHERE person.first_name NOT LIKE 'A!_%' ESCAPE '!'"#, + ); + + sql_round_trip( + GenericDialect {}, + r#"SELECT * FROM person WHERE first_name NOT ILIKE 'A!_%' ESCAPE '!'"#, + r#"SELECT * FROM person WHERE person.first_name NOT ILIKE 'A!_%' ESCAPE '!'"#, + ); } #[test]