You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "beliefer (via GitHub)" <gi...@apache.org> on 2023/11/14 13:30:23 UTC
[PR] [SPARK-38432][SQL][FOLLOWUP] Escape the quote and % for DS V2 pushdown [spark]
beliefer opened a new pull request, #43801:
URL: https://github.com/apache/spark/pull/43801
### What changes were proposed in this pull request?
Spark supports push down `startsWith`, `endWith` and `contains` to JDBC database with DS V2 pushdown.
But the V2ExpressionSQLBuilder didn't escape the quote and %, it can cause unexpected result.
### Why are the changes needed?
Escape the quote and % for DS V2 pushdown
### Does this PR introduce _any_ user-facing change?
'No'.
### How was this patch tested?
Exists test cases.
### Was this patch authored or co-authored using generative AI tooling?
'No'.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "JoshRosen (via GitHub)" <gi...@apache.org>.
JoshRosen commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400156026
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -238,21 +261,21 @@ protected String visitStartsWith(String l, String r) {
// Remove quotes at the beginning and end.
// e.g. converts "'str'" to "str".
String value = r.substring(1, r.length() - 1);
- return l + " LIKE '" + value + "%'";
+ return l + " LIKE '" + escapeSpecialChar(value) + "%'";
Review Comment:
We might need to add an `ESCAPE "\\"` to explicitly specify the escape character to use in the `LIKE` because I don't know that all databases have a default escape character.
For example, from [Oracle docs](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/Pattern-matching-Conditions.html#GUID-0779657B-06A8-441F-90C5-044B47862A0A):
> If `esc_char` is not specified, then there is no default escape character.
And [Snowflake](https://docs.snowflake.com/en/sql-reference/functions/like#usage-notes):
> There is no default escape character.
> If you use the backslash as an escape character, then you must specify escape the backslash in the ESCAPE clause. For example, the following command specifies that the escape character is the backslash, and then uses that escape character to search for ‘%’ as a literal (without the escape character, the ‘%’ would be treated as a wildcard):
> `'SOMETHING%' LIKE '%\\%%' ESCAPE '\\';`
[T-SQL](https://learn.microsoft.com/en-us/sql/t-sql/language-elements/like-transact-sql) says that it supports `LIKE` but that
> ESCAPE and STRING_ESCAPE are not supported in Azure Synapse Analytics or Analytics Platform System (PDW).
and also seems to [suggest](https://learn.microsoft.com/en-us/sql/t-sql/language-elements/like-transact-sql?view=sql-server-ver16#pattern-match-with-the-escape-clause) that `ESCAPE` must always be specified when using escape characters:
> To search for the percent sign as a character instead of as a wildcard character, the ESCAPE keyword and escape character must be provided
---
Given all of this, I am wondering whether we might need to allow the pushdown implementation to vary on a per-SQL-dialect basis, or, alternatively, have some way to skip pushdown of these expressions in cases where they need to be escaped but the underlying database does not support the `ESCAPE` clause.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -238,21 +261,21 @@ protected String visitStartsWith(String l, String r) {
// Remove quotes at the beginning and end.
// e.g. converts "'str'" to "str".
String value = r.substring(1, r.length() - 1);
- return l + " LIKE '" + value + "%'";
+ return l + " LIKE '" + escapeSpecialChar(value) + "%'";
Review Comment:
We might need to add an `ESCAPE '\\'` to explicitly specify the escape character to use in the `LIKE` because I don't know that all databases have a default escape character.
For example, from [Oracle docs](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/Pattern-matching-Conditions.html#GUID-0779657B-06A8-441F-90C5-044B47862A0A):
> If `esc_char` is not specified, then there is no default escape character.
And [Snowflake](https://docs.snowflake.com/en/sql-reference/functions/like#usage-notes):
> There is no default escape character.
> If you use the backslash as an escape character, then you must specify escape the backslash in the ESCAPE clause. For example, the following command specifies that the escape character is the backslash, and then uses that escape character to search for ‘%’ as a literal (without the escape character, the ‘%’ would be treated as a wildcard):
> `'SOMETHING%' LIKE '%\\%%' ESCAPE '\\';`
[T-SQL](https://learn.microsoft.com/en-us/sql/t-sql/language-elements/like-transact-sql) says that it supports `LIKE` but that
> ESCAPE and STRING_ESCAPE are not supported in Azure Synapse Analytics or Analytics Platform System (PDW).
and also seems to [suggest](https://learn.microsoft.com/en-us/sql/t-sql/language-elements/like-transact-sql?view=sql-server-ver16#pattern-match-with-the-escape-clause) that `ESCAPE` must always be specified when using escape characters:
> To search for the percent sign as a character instead of as a wildcard character, the ESCAPE keyword and escape character must be provided
---
Given all of this, I am wondering whether we might need to allow the pushdown implementation to vary on a per-SQL-dialect basis, or, alternatively, have some way to skip pushdown of these expressions in cases where they need to be escaped but the underlying database does not support the `ESCAPE` clause.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400224757
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +48,29 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialChar(String str) {
+ StringBuilder builder = new StringBuilder();
+
+ for (char c : str.toCharArray()) {
+ switch (c) {
+ case '_':
+ builder.append("\\_");
+ break;
+ case '%':
+ builder.append("\\%");
+ break;
+ case '\'':
+ builder.append("\'");
Review Comment:
`JDBCSQLBuilder.compileValue` calls `escapeSql`.
https://github.com/apache/spark/blob/0b7a537be557032abddf42f143a079889a584c18/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala#L289
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1408579624
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/H2Dialect.scala:
##########
@@ -242,6 +242,14 @@ private[sql] object H2Dialect extends JdbcDialect {
}
class H2SQLBuilder extends JDBCSQLBuilder {
+ override def escapeSpecialCharsForLikePattern(str: String): String = {
+ str.map {
+ case '_' => "\\_"
+ case '%' => "\\%"
+ case c => c.toString
Review Comment:
H2 does not need to escape `\`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #43801:
URL: https://github.com/apache/spark/pull/43801#issuecomment-1831004213
thanks, merging to master!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400156345
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +48,29 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialChar(String str) {
+ StringBuilder builder = new StringBuilder();
+
+ for (char c : str.toCharArray()) {
+ switch (c) {
+ case '_':
+ builder.append("\\_");
+ break;
+ case '%':
+ builder.append("\\%");
+ break;
+ case '\'':
+ builder.append("\'");
Review Comment:
> spark SQL will escape the single quote.
Where does it happen?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400230963
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -238,21 +261,21 @@ protected String visitStartsWith(String l, String r) {
// Remove quotes at the beginning and end.
// e.g. converts "'str'" to "str".
String value = r.substring(1, r.length() - 1);
- return l + " LIKE '" + value + "%'";
+ return l + " LIKE '" + escapeSpecialChar(value) + "%'";
Review Comment:
+1 to explicitly add the `ESCAPT` clause. The worse case is the underling database doesn't support it and fail, which is better than getting wrong result silently.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1403333706
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala:
##########
@@ -1262,6 +1275,94 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkAnswer(df17, Seq(Row(6, "jen", 12000, 1200, true)))
}
+ test("SPARK-38432: escape the single quote, _ and % for DS V2 pushdown") {
+ val df1 = spark.table("h2.test.address").filter($"email".startsWith("abc_"))
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_%' ESCAPE '\']")
+ checkAnswer(df1,
+ Seq(Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df2 = spark.table("h2.test.address").filter($"email".startsWith("abc%"))
+ checkFiltersRemoved(df2)
+ checkPushedInfo(df2, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\%%' ESCAPE '\']")
+ checkAnswer(df2, Seq(Row("abc%_def@gmail.com"), Row("abc%def@gmail.com")))
+
+ val df3 = spark.table("h2.test.address").filter($"email".startsWith("abc%_"))
+ checkFiltersRemoved(df3)
+ checkPushedInfo(df3, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\%\_%' ESCAPE '\']")
+ checkAnswer(df3, Seq(Row("abc%_def@gmail.com")))
+
+ val df4 = spark.table("h2.test.address").filter($"email".startsWith("abc_%"))
+ checkFiltersRemoved(df4)
+ checkPushedInfo(df4, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_\%%' ESCAPE '\']")
+ checkAnswer(df4, Seq(Row("abc_%def@gmail.com")))
+
+ val df5 = spark.table("h2.test.address").filter($"email".startsWith("abc_'%"))
+ checkFiltersRemoved(df5)
+ checkPushedInfo(df5,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_'\%%' ESCAPE '\']")
+ checkAnswer(df5, Seq(Row("abc_'%def@gmail.com")))
+
+ val df6 = spark.table("h2.test.address").filter($"email".endsWith("_def@gmail.com"))
+ checkFiltersRemoved(df6)
+ checkPushedInfo(df6,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_def@gmail.com' ESCAPE '\']")
+ checkAnswer(df6, Seq(Row("abc%_def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df7 = spark.table("h2.test.address").filter($"email".endsWith("%def@gmail.com"))
+ checkFiltersRemoved(df7)
+ checkPushedInfo(df7,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df7,
+ Seq(Row("abc%def@gmail.com"), Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com")))
+
+ val df8 = spark.table("h2.test.address").filter($"email".endsWith("%_def@gmail.com"))
+ checkFiltersRemoved(df8)
+ checkPushedInfo(df8,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\%\_def@gmail.com' ESCAPE '\']")
+ checkAnswer(df8, Seq(Row("abc%_def@gmail.com")))
+
+ val df9 = spark.table("h2.test.address").filter($"email".endsWith("_%def@gmail.com"))
+ checkFiltersRemoved(df9)
+ checkPushedInfo(df9,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df9, Seq(Row("abc_%def@gmail.com")))
+
+ val df10 = spark.table("h2.test.address").filter($"email".endsWith("_'%def@gmail.com"))
+ checkFiltersRemoved(df10)
+ checkPushedInfo(df10,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_'\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df10, Seq(Row("abc_'%def@gmail.com")))
+
+ val df11 = spark.table("h2.test.address").filter($"email".contains("c_d"))
+ checkFiltersRemoved(df11)
+ checkPushedInfo(df11, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_d%' ESCAPE '\']")
+ checkAnswer(df11, Seq(Row("abc_def@gmail.com")))
+
+ val df12 = spark.table("h2.test.address").filter($"email".contains("c%d"))
+ checkFiltersRemoved(df12)
+ checkPushedInfo(df12, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\%d%' ESCAPE '\']")
+ checkAnswer(df12, Seq(Row("abc%def@gmail.com")))
+
+ val df13 = spark.table("h2.test.address").filter($"email".contains("c%_d"))
+ checkFiltersRemoved(df13)
+ checkPushedInfo(df13,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\%\_d%' ESCAPE '\']")
+ checkAnswer(df13, Seq(Row("abc%_def@gmail.com")))
+
+ val df14 = spark.table("h2.test.address").filter($"email".contains("c_%d"))
+ checkFiltersRemoved(df14)
+ checkPushedInfo(df14,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_\%d%' ESCAPE '\']")
+ checkAnswer(df14, Seq(Row("abc_%def@gmail.com")))
+
+ val df15 = spark.table("h2.test.address").filter($"email".contains("c_'%d"))
+ checkFiltersRemoved(df15)
+ checkPushedInfo(df15,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_'\%d%' ESCAPE '\']")
Review Comment:
Shouldn't it be `'%c\_''\%d%'`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1399950510
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +49,29 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialChar(String str) {
+ StringBuilder builder = new StringBuilder();
+
+ for (char c : str.toCharArray()) {
+ switch (c) {
Review Comment:
I tested in java8, it works good.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400121955
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +48,29 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialChar(String str) {
+ StringBuilder builder = new StringBuilder();
+
+ for (char c : str.toCharArray()) {
+ switch (c) {
+ case '_':
+ builder.append("\\_");
+ break;
+ case '%':
+ builder.append("\\%");
+ break;
+ case '\'':
+ builder.append("\'");
Review Comment:
H2 doesn't support it.
```
org.h2.jdbc.JdbcSQLSyntaxErrorException: Syntax error in SQL statement "SELECT ""EMAIL"" FROM ""test"".""address"" WHERE (""EMAIL"" IS NOT NULL) AND (""EMAIL"" LIKE 'abc\\_\\'[*]\\'\\%%') "; SQL statement:
SELECT "EMAIL" FROM "test"."address" WHERE ("EMAIL" IS NOT NULL) AND ("EMAIL" LIKE 'abc\_\'\'\%%') [42000-220]
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400133212
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala:
##########
@@ -1262,6 +1275,85 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkAnswer(df17, Seq(Row(6, "jen", 12000, 1200, true)))
}
+ test("SPARK-38432: escape the quote, _ and % for DS V2 pushdown") {
+ val df1 = spark.table("h2.test.address").filter($"email".startsWith("abc_"))
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_%']")
+ checkAnswer(df1,
+ Seq(Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df2 = spark.table("h2.test.address").filter($"email".startsWith("abc%"))
+ checkFiltersRemoved(df2)
+ checkPushedInfo(df2, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%%']")
+ checkAnswer(df2, Seq(Row("abc%_def@gmail.com"), Row("abc%def@gmail.com")))
+
+ val df3 = spark.table("h2.test.address").filter($"email".startsWith("abc%_"))
+ checkFiltersRemoved(df3)
+ checkPushedInfo(df3, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%\\_%']")
+ checkAnswer(df3, Seq(Row("abc%_def@gmail.com")))
+
+ val df4 = spark.table("h2.test.address").filter($"email".startsWith("abc_%"))
+ checkFiltersRemoved(df4)
+ checkPushedInfo(df4, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_\\%%']")
+ checkAnswer(df4, Seq(Row("abc_%def@gmail.com")))
+
+ val df5 = spark.table("h2.test.address").filter($"email".startsWith("abc_'%"))
+ checkFiltersRemoved(df5)
+ checkPushedInfo(df5, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_'\\%%']")
Review Comment:
In fact, DS V2 pass `"EMAIL" LIKE 'abc\_''\%%'` to H2.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1403363778
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala:
##########
@@ -1262,6 +1275,94 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkAnswer(df17, Seq(Row(6, "jen", 12000, 1200, true)))
}
+ test("SPARK-38432: escape the single quote, _ and % for DS V2 pushdown") {
+ val df1 = spark.table("h2.test.address").filter($"email".startsWith("abc_"))
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_%' ESCAPE '\']")
+ checkAnswer(df1,
+ Seq(Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df2 = spark.table("h2.test.address").filter($"email".startsWith("abc%"))
+ checkFiltersRemoved(df2)
+ checkPushedInfo(df2, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\%%' ESCAPE '\']")
+ checkAnswer(df2, Seq(Row("abc%_def@gmail.com"), Row("abc%def@gmail.com")))
+
+ val df3 = spark.table("h2.test.address").filter($"email".startsWith("abc%_"))
+ checkFiltersRemoved(df3)
+ checkPushedInfo(df3, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\%\_%' ESCAPE '\']")
+ checkAnswer(df3, Seq(Row("abc%_def@gmail.com")))
+
+ val df4 = spark.table("h2.test.address").filter($"email".startsWith("abc_%"))
+ checkFiltersRemoved(df4)
+ checkPushedInfo(df4, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_\%%' ESCAPE '\']")
+ checkAnswer(df4, Seq(Row("abc_%def@gmail.com")))
+
+ val df5 = spark.table("h2.test.address").filter($"email".startsWith("abc_'%"))
+ checkFiltersRemoved(df5)
+ checkPushedInfo(df5,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_'\%%' ESCAPE '\']")
+ checkAnswer(df5, Seq(Row("abc_'%def@gmail.com")))
+
+ val df6 = spark.table("h2.test.address").filter($"email".endsWith("_def@gmail.com"))
+ checkFiltersRemoved(df6)
+ checkPushedInfo(df6,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_def@gmail.com' ESCAPE '\']")
+ checkAnswer(df6, Seq(Row("abc%_def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df7 = spark.table("h2.test.address").filter($"email".endsWith("%def@gmail.com"))
+ checkFiltersRemoved(df7)
+ checkPushedInfo(df7,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df7,
+ Seq(Row("abc%def@gmail.com"), Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com")))
+
+ val df8 = spark.table("h2.test.address").filter($"email".endsWith("%_def@gmail.com"))
+ checkFiltersRemoved(df8)
+ checkPushedInfo(df8,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\%\_def@gmail.com' ESCAPE '\']")
+ checkAnswer(df8, Seq(Row("abc%_def@gmail.com")))
+
+ val df9 = spark.table("h2.test.address").filter($"email".endsWith("_%def@gmail.com"))
+ checkFiltersRemoved(df9)
+ checkPushedInfo(df9,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df9, Seq(Row("abc_%def@gmail.com")))
+
+ val df10 = spark.table("h2.test.address").filter($"email".endsWith("_'%def@gmail.com"))
+ checkFiltersRemoved(df10)
+ checkPushedInfo(df10,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_'\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df10, Seq(Row("abc_'%def@gmail.com")))
+
+ val df11 = spark.table("h2.test.address").filter($"email".contains("c_d"))
+ checkFiltersRemoved(df11)
+ checkPushedInfo(df11, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_d%' ESCAPE '\']")
+ checkAnswer(df11, Seq(Row("abc_def@gmail.com")))
+
+ val df12 = spark.table("h2.test.address").filter($"email".contains("c%d"))
+ checkFiltersRemoved(df12)
+ checkPushedInfo(df12, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\%d%' ESCAPE '\']")
+ checkAnswer(df12, Seq(Row("abc%def@gmail.com")))
+
+ val df13 = spark.table("h2.test.address").filter($"email".contains("c%_d"))
+ checkFiltersRemoved(df13)
+ checkPushedInfo(df13,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\%\_d%' ESCAPE '\']")
+ checkAnswer(df13, Seq(Row("abc%_def@gmail.com")))
+
+ val df14 = spark.table("h2.test.address").filter($"email".contains("c_%d"))
+ checkFiltersRemoved(df14)
+ checkPushedInfo(df14,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_\%d%' ESCAPE '\']")
+ checkAnswer(df14, Seq(Row("abc_%def@gmail.com")))
+
+ val df15 = spark.table("h2.test.address").filter($"email".contains("c_'%d"))
+ checkFiltersRemoved(df15)
+ checkPushedInfo(df15,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_'\%d%' ESCAPE '\']")
Review Comment:
I'm surprised the test passed. When we call `visitContains`, the like pattern was produced by `visitLiteral` which invokes `compileValue` to escape the `'`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1399259589
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +49,29 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialChar(String str) {
+ StringBuilder builder = new StringBuilder();
+
+ for (char c : str.toCharArray()) {
+ switch (c) {
Review Comment:
just to double-check: does this syntax work in java 8? This is a correctness bug and we need to backport it, and only the master branch has dropped java 8 support.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1398653003
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/QuotingUtils.scala:
##########
@@ -71,4 +71,17 @@ object QuotingUtils {
builder.toString()
}
+
+ def escapeSpecialChar(str: String): String = {
+ val builder = new StringBuilder
+
+ str.foreach {
Review Comment:
isn't it just `str.map`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1399950925
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala:
##########
@@ -1262,6 +1275,85 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkAnswer(df17, Seq(Row(6, "jen", 12000, 1200, true)))
}
+ test("SPARK-38432: escape the quote, _ and % for DS V2 pushdown") {
+ val df1 = spark.table("h2.test.address").filter($"email".startsWith("abc_"))
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_%']")
+ checkAnswer(df1,
+ Seq(Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df2 = spark.table("h2.test.address").filter($"email".startsWith("abc%"))
+ checkFiltersRemoved(df2)
+ checkPushedInfo(df2, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%%']")
+ checkAnswer(df2, Seq(Row("abc%_def@gmail.com"), Row("abc%def@gmail.com")))
+
+ val df3 = spark.table("h2.test.address").filter($"email".startsWith("abc%_"))
+ checkFiltersRemoved(df3)
+ checkPushedInfo(df3, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%\\_%']")
+ checkAnswer(df3, Seq(Row("abc%_def@gmail.com")))
+
+ val df4 = spark.table("h2.test.address").filter($"email".startsWith("abc_%"))
+ checkFiltersRemoved(df4)
+ checkPushedInfo(df4, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_\\%%']")
+ checkAnswer(df4, Seq(Row("abc_%def@gmail.com")))
+
+ val df5 = spark.table("h2.test.address").filter($"email".startsWith("abc_'%"))
+ checkFiltersRemoved(df5)
+ checkPushedInfo(df5, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_'\\%%']")
Review Comment:
I don't know the background, but it works good.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #43801: [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown
URL: https://github.com/apache/spark/pull/43801
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1399964550
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala:
##########
@@ -1262,6 +1275,85 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkAnswer(df17, Seq(Row(6, "jen", 12000, 1200, true)))
}
+ test("SPARK-38432: escape the quote, _ and % for DS V2 pushdown") {
+ val df1 = spark.table("h2.test.address").filter($"email".startsWith("abc_"))
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_%']")
+ checkAnswer(df1,
+ Seq(Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df2 = spark.table("h2.test.address").filter($"email".startsWith("abc%"))
+ checkFiltersRemoved(df2)
+ checkPushedInfo(df2, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%%']")
+ checkAnswer(df2, Seq(Row("abc%_def@gmail.com"), Row("abc%def@gmail.com")))
+
+ val df3 = spark.table("h2.test.address").filter($"email".startsWith("abc%_"))
+ checkFiltersRemoved(df3)
+ checkPushedInfo(df3, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%\\_%']")
+ checkAnswer(df3, Seq(Row("abc%_def@gmail.com")))
+
+ val df4 = spark.table("h2.test.address").filter($"email".startsWith("abc_%"))
+ checkFiltersRemoved(df4)
+ checkPushedInfo(df4, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_\\%%']")
+ checkAnswer(df4, Seq(Row("abc_%def@gmail.com")))
+
+ val df5 = spark.table("h2.test.address").filter($"email".startsWith("abc_'%"))
+ checkFiltersRemoved(df5)
+ checkPushedInfo(df5, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_'\\%%']")
Review Comment:
At least snowflake asks you to quote it: https://docs.snowflake.com/en/sql-reference/data-types-text#escape-sequences-in-single-quoted-string-constants
Are you sure this works for all databases not only H2?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400092530
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala:
##########
@@ -1262,6 +1275,85 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkAnswer(df17, Seq(Row(6, "jen", 12000, 1200, true)))
}
+ test("SPARK-38432: escape the quote, _ and % for DS V2 pushdown") {
+ val df1 = spark.table("h2.test.address").filter($"email".startsWith("abc_"))
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_%']")
+ checkAnswer(df1,
+ Seq(Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df2 = spark.table("h2.test.address").filter($"email".startsWith("abc%"))
+ checkFiltersRemoved(df2)
+ checkPushedInfo(df2, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%%']")
+ checkAnswer(df2, Seq(Row("abc%_def@gmail.com"), Row("abc%def@gmail.com")))
+
+ val df3 = spark.table("h2.test.address").filter($"email".startsWith("abc%_"))
+ checkFiltersRemoved(df3)
+ checkPushedInfo(df3, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%\\_%']")
+ checkAnswer(df3, Seq(Row("abc%_def@gmail.com")))
+
+ val df4 = spark.table("h2.test.address").filter($"email".startsWith("abc_%"))
+ checkFiltersRemoved(df4)
+ checkPushedInfo(df4, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_\\%%']")
+ checkAnswer(df4, Seq(Row("abc_%def@gmail.com")))
+
+ val df5 = spark.table("h2.test.address").filter($"email".startsWith("abc_'%"))
+ checkFiltersRemoved(df5)
+ checkPushedInfo(df5, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_'\\%%']")
Review Comment:
then let's quote it like other special chars to be safe.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "JoshRosen (via GitHub)" <gi...@apache.org>.
JoshRosen commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400156026
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -238,21 +261,21 @@ protected String visitStartsWith(String l, String r) {
// Remove quotes at the beginning and end.
// e.g. converts "'str'" to "str".
String value = r.substring(1, r.length() - 1);
- return l + " LIKE '" + value + "%'";
+ return l + " LIKE '" + escapeSpecialChar(value) + "%'";
Review Comment:
We might need to add an `ESCAPE "\"` to explicitly specify the escape character to use in the `LIKE` because I don't know that all databases have a default escape character.
For example, from [Oracle docs](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/Pattern-matching-Conditions.html#GUID-0779657B-06A8-441F-90C5-044B47862A0A):
> If `esc_char` is not specified, then there is no default escape character.
And [Snowflake](https://docs.snowflake.com/en/sql-reference/functions/like#usage-notes):
> There is no default escape character.
> If you use the backslash as an escape character, then you must specify escape the backslash in the ESCAPE clause. For example, the following command specifies that the escape character is the backslash, and then uses that escape character to search for ‘%’ as a literal (without the escape character, the ‘%’ would be treated as a wildcard):
> `'SOMETHING%' LIKE '%\\%%' ESCAPE '\\';`
[T-SQL](https://learn.microsoft.com/en-us/sql/t-sql/language-elements/like-transact-sql) says that it supports `LIKE` but that
> ESCAPE and STRING_ESCAPE are not supported in Azure Synapse Analytics or Analytics Platform System (PDW).
and also seems to [suggest](https://learn.microsoft.com/en-us/sql/t-sql/language-elements/like-transact-sql?view=sql-server-ver16#pattern-match-with-the-escape-clause) that `ESCAPE` must always be specified when using escape characters:
> To search for the percent sign as a character instead of as a wildcard character, the ESCAPE keyword and escape character must be provided
---
Given all of this, I am wondering whether we might need to allow the pushdown implementation to vary on a per-SQL-dialect basis, or, alternatively, have some way to skip pushdown of these expressions in cases where they need to be escaped but the underlying database does not support the `ESCAPE` clause.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -238,21 +261,21 @@ protected String visitStartsWith(String l, String r) {
// Remove quotes at the beginning and end.
// e.g. converts "'str'" to "str".
String value = r.substring(1, r.length() - 1);
- return l + " LIKE '" + value + "%'";
+ return l + " LIKE '" + escapeSpecialChar(value) + "%'";
Review Comment:
We might need to add an `ESCAPE "\"` to explicitly specify the escape character to use in the `LIKE` because I don't know that all databases have a default escape character.
For example, from [Oracle docs](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/Pattern-matching-Conditions.html#GUID-0779657B-06A8-441F-90C5-044B47862A0A):
> If `esc_char` is not specified, then there is no default escape character.
And [Snowflake](https://docs.snowflake.com/en/sql-reference/functions/like#usage-notes):
> There is no default escape character.
> If you use the backslash as an escape character, then you must specify escape the backslash in the ESCAPE clause. For example, the following command specifies that the escape character is the backslash, and then uses that escape character to search for ‘%’ as a literal (without the escape character, the ‘%’ would be treated as a wildcard):
> `'SOMETHING%' LIKE '%\\%%' ESCAPE '\\';`
[T-SQL](https://learn.microsoft.com/en-us/sql/t-sql/language-elements/like-transact-sql) says that it supports `LIKE` but that
> ESCAPE and STRING_ESCAPE are not supported in Azure Synapse Analytics or Analytics Platform System (PDW).
and also seems to [suggest](https://learn.microsoft.com/en-us/sql/t-sql/language-elements/like-transact-sql?view=sql-server-ver16#pattern-match-with-the-escape-clause) that `ESCAPE` must always be specified when using escape characters:
> To search for the percent sign as a character instead of as a wildcard character, the ESCAPE keyword and escape character must be provided
---
Given all of this, I am wondering whether we might need to allow the pushdown implementation to vary on a per-SQL-dialect basis, or, alternatively, have some way to skip pushdown of these expressions in cases where they need to be escaped but the underlying database does not support the `ESCAPE` clause.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #43801:
URL: https://github.com/apache/spark/pull/43801#issuecomment-1831006009
@beliefer can you open backport PRs for 3.4 and 3.3? Thanks!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #43801:
URL: https://github.com/apache/spark/pull/43801#issuecomment-1823960865
@JoshRosen @cloud-fan Please help to review this PR again.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1403843369
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala:
##########
@@ -1262,6 +1275,94 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkAnswer(df17, Seq(Row(6, "jen", 12000, 1200, true)))
}
+ test("SPARK-38432: escape the single quote, _ and % for DS V2 pushdown") {
+ val df1 = spark.table("h2.test.address").filter($"email".startsWith("abc_"))
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_%' ESCAPE '\']")
+ checkAnswer(df1,
+ Seq(Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df2 = spark.table("h2.test.address").filter($"email".startsWith("abc%"))
+ checkFiltersRemoved(df2)
+ checkPushedInfo(df2, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\%%' ESCAPE '\']")
+ checkAnswer(df2, Seq(Row("abc%_def@gmail.com"), Row("abc%def@gmail.com")))
+
+ val df3 = spark.table("h2.test.address").filter($"email".startsWith("abc%_"))
+ checkFiltersRemoved(df3)
+ checkPushedInfo(df3, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\%\_%' ESCAPE '\']")
+ checkAnswer(df3, Seq(Row("abc%_def@gmail.com")))
+
+ val df4 = spark.table("h2.test.address").filter($"email".startsWith("abc_%"))
+ checkFiltersRemoved(df4)
+ checkPushedInfo(df4, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_\%%' ESCAPE '\']")
+ checkAnswer(df4, Seq(Row("abc_%def@gmail.com")))
+
+ val df5 = spark.table("h2.test.address").filter($"email".startsWith("abc_'%"))
+ checkFiltersRemoved(df5)
+ checkPushedInfo(df5,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_'\%%' ESCAPE '\']")
+ checkAnswer(df5, Seq(Row("abc_'%def@gmail.com")))
+
+ val df6 = spark.table("h2.test.address").filter($"email".endsWith("_def@gmail.com"))
+ checkFiltersRemoved(df6)
+ checkPushedInfo(df6,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_def@gmail.com' ESCAPE '\']")
+ checkAnswer(df6, Seq(Row("abc%_def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df7 = spark.table("h2.test.address").filter($"email".endsWith("%def@gmail.com"))
+ checkFiltersRemoved(df7)
+ checkPushedInfo(df7,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df7,
+ Seq(Row("abc%def@gmail.com"), Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com")))
+
+ val df8 = spark.table("h2.test.address").filter($"email".endsWith("%_def@gmail.com"))
+ checkFiltersRemoved(df8)
+ checkPushedInfo(df8,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\%\_def@gmail.com' ESCAPE '\']")
+ checkAnswer(df8, Seq(Row("abc%_def@gmail.com")))
+
+ val df9 = spark.table("h2.test.address").filter($"email".endsWith("_%def@gmail.com"))
+ checkFiltersRemoved(df9)
+ checkPushedInfo(df9,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df9, Seq(Row("abc_%def@gmail.com")))
+
+ val df10 = spark.table("h2.test.address").filter($"email".endsWith("_'%def@gmail.com"))
+ checkFiltersRemoved(df10)
+ checkPushedInfo(df10,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_'\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df10, Seq(Row("abc_'%def@gmail.com")))
+
+ val df11 = spark.table("h2.test.address").filter($"email".contains("c_d"))
+ checkFiltersRemoved(df11)
+ checkPushedInfo(df11, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_d%' ESCAPE '\']")
+ checkAnswer(df11, Seq(Row("abc_def@gmail.com")))
+
+ val df12 = spark.table("h2.test.address").filter($"email".contains("c%d"))
+ checkFiltersRemoved(df12)
+ checkPushedInfo(df12, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\%d%' ESCAPE '\']")
+ checkAnswer(df12, Seq(Row("abc%def@gmail.com")))
+
+ val df13 = spark.table("h2.test.address").filter($"email".contains("c%_d"))
+ checkFiltersRemoved(df13)
+ checkPushedInfo(df13,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\%\_d%' ESCAPE '\']")
+ checkAnswer(df13, Seq(Row("abc%_def@gmail.com")))
+
+ val df14 = spark.table("h2.test.address").filter($"email".contains("c_%d"))
+ checkFiltersRemoved(df14)
+ checkPushedInfo(df14,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_\%d%' ESCAPE '\']")
+ checkAnswer(df14, Seq(Row("abc_%def@gmail.com")))
+
+ val df15 = spark.table("h2.test.address").filter($"email".contains("c_'%d"))
+ checkFiltersRemoved(df15)
+ checkPushedInfo(df15,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_'\%d%' ESCAPE '\']")
Review Comment:
The reason is the pushed info is related to the DS V2 expressions. The `compileValue` is related to the JDBC dialect.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1398653848
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/QuotingUtils.scala:
##########
@@ -71,4 +71,17 @@ object QuotingUtils {
builder.toString()
}
+
+ def escapeSpecialChar(str: String): String = {
+ val builder = new StringBuilder
+
+ str.foreach {
+ case '_' => builder ++= "\\_"
+ case '%' => builder ++= "\\%"
+ // TODO Support for escaping more special characters
Review Comment:
I think we should at least also escape `'`, as the generated sql is like `" LIKE '" + value + "%'";`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "JoshRosen (via GitHub)" <gi...@apache.org>.
JoshRosen commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400158114
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +48,29 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialChar(String str) {
Review Comment:
Perhaps we should name this `escapeSpecialCharsForLikePattern` or similar, since this isn't a fully generalized escaping mechanism?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400437832
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +48,26 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialCharsForLikePattern(String str) {
+ StringBuilder builder = new StringBuilder();
+
+ for (char c : str.toCharArray()) {
+ switch (c) {
+ case '_':
+ builder.append("\\_");
+ break;
+ case '%':
+ builder.append("\\%");
+ break;
+ // TODO Support for escaping more special characters
Review Comment:
It's not a TODO. We should document `escapeSpecialCharsForLikePattern` and ask dialects to override it if they have more special chars other than `_` and `%`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400129296
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +48,29 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialChar(String str) {
+ StringBuilder builder = new StringBuilder();
+
+ for (char c : str.toCharArray()) {
+ switch (c) {
+ case '_':
+ builder.append("\\_");
+ break;
+ case '%':
+ builder.append("\\%");
+ break;
+ case '\'':
+ builder.append("\'");
Review Comment:
For `$"email".startsWith("abc_'%")`, spark SQL will escape the single quote.
So the DS V2 will get the input `abc_''%`.
H2 also supports use `'` to escape `'`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1398653431
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/QuotingUtils.scala:
##########
@@ -71,4 +71,17 @@ object QuotingUtils {
builder.toString()
}
+
+ def escapeSpecialChar(str: String): String = {
Review Comment:
shall we put this function in `V2ExpressionSQLBuilder`? Maybe some dialects want to override it as they have more special chars.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400121955
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +48,29 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialChar(String str) {
+ StringBuilder builder = new StringBuilder();
+
+ for (char c : str.toCharArray()) {
+ switch (c) {
+ case '_':
+ builder.append("\\_");
+ break;
+ case '%':
+ builder.append("\\%");
+ break;
+ case '\'':
+ builder.append("\'");
Review Comment:
H2 doesn't support it.
```
org.h2.jdbc.JdbcSQLSyntaxErrorException: Syntax error in SQL statement "SELECT ""EMAIL"" FROM ""test"".""address"" WHERE (""EMAIL"" IS NOT NULL) AND (""EMAIL"" LIKE 'abc\\_\\'[*]\\'\\%%') "; SQL statement:
SELECT "EMAIL" FROM "test"."address" WHERE ("EMAIL" IS NOT NULL) AND ("EMAIL" LIKE 'abc\_\'\'\%%') [42000-220]
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #43801:
URL: https://github.com/apache/spark/pull/43801#issuecomment-1822182897
@cloud-fan The GA failure is unrelated.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400229619
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +48,29 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialChar(String str) {
+ StringBuilder builder = new StringBuilder();
+
+ for (char c : str.toCharArray()) {
+ switch (c) {
+ case '_':
+ builder.append("\\_");
+ break;
+ case '%':
+ builder.append("\\%");
+ break;
+ case '\'':
+ builder.append("\'");
Review Comment:
ah good to know. Then we can remove `case '\'':`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400172171
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -238,21 +261,21 @@ protected String visitStartsWith(String l, String r) {
// Remove quotes at the beginning and end.
// e.g. converts "'str'" to "str".
String value = r.substring(1, r.length() - 1);
- return l + " LIKE '" + value + "%'";
+ return l + " LIKE '" + escapeSpecialChar(value) + "%'";
Review Comment:
Yes. As you said, DS V2 pushdown framework allow SQL-dialect to overwrite the `V2ExpressionSQLBuilder .escapeSpecialChar`. For example, the built-in `H2SQLBuilder` extends `V2ExpressionSQLBuilder`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1399965784
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala:
##########
@@ -1262,6 +1275,85 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkAnswer(df17, Seq(Row(6, "jen", 12000, 1200, true)))
}
+ test("SPARK-38432: escape the quote, _ and % for DS V2 pushdown") {
+ val df1 = spark.table("h2.test.address").filter($"email".startsWith("abc_"))
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_%']")
Review Comment:
to make the tests more readable, let use `raw"..."` so that we don't need to write `\\` but just `\`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400121955
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +48,29 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialChar(String str) {
+ StringBuilder builder = new StringBuilder();
+
+ for (char c : str.toCharArray()) {
+ switch (c) {
+ case '_':
+ builder.append("\\_");
+ break;
+ case '%':
+ builder.append("\\%");
+ break;
+ case '\'':
+ builder.append("\'");
Review Comment:
H2 doesn't it.
```
org.h2.jdbc.JdbcSQLSyntaxErrorException: Syntax error in SQL statement "SELECT ""EMAIL"" FROM ""test"".""address"" WHERE (""EMAIL"" IS NOT NULL) AND (""EMAIL"" LIKE 'abc\\_\\'[*]\\'\\%%') "; SQL statement:
SELECT "EMAIL" FROM "test"."address" WHERE ("EMAIL" IS NOT NULL) AND ("EMAIL" LIKE 'abc\_\'\'\%%') [42000-220]
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #43801:
URL: https://github.com/apache/spark/pull/43801#issuecomment-1825321263
The GA failure is unrelated.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #43801:
URL: https://github.com/apache/spark/pull/43801#issuecomment-1816056265
ping @cloud-fan @huaxingao
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400266068
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -238,21 +261,21 @@ protected String visitStartsWith(String l, String r) {
// Remove quotes at the beginning and end.
// e.g. converts "'str'" to "str".
String value = r.substring(1, r.length() - 1);
- return l + " LIKE '" + value + "%'";
+ return l + " LIKE '" + escapeSpecialChar(value) + "%'";
Review Comment:
Sounds good to me.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1399261265
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala:
##########
@@ -1262,6 +1275,85 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkAnswer(df17, Seq(Row(6, "jen", 12000, 1200, true)))
}
+ test("SPARK-38432: escape the quote, _ and % for DS V2 pushdown") {
+ val df1 = spark.table("h2.test.address").filter($"email".startsWith("abc_"))
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_%']")
+ checkAnswer(df1,
+ Seq(Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df2 = spark.table("h2.test.address").filter($"email".startsWith("abc%"))
+ checkFiltersRemoved(df2)
+ checkPushedInfo(df2, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%%']")
+ checkAnswer(df2, Seq(Row("abc%_def@gmail.com"), Row("abc%def@gmail.com")))
+
+ val df3 = spark.table("h2.test.address").filter($"email".startsWith("abc%_"))
+ checkFiltersRemoved(df3)
+ checkPushedInfo(df3, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%\\_%']")
+ checkAnswer(df3, Seq(Row("abc%_def@gmail.com")))
+
+ val df4 = spark.table("h2.test.address").filter($"email".startsWith("abc_%"))
+ checkFiltersRemoved(df4)
+ checkPushedInfo(df4, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_\\%%']")
+ checkAnswer(df4, Seq(Row("abc_%def@gmail.com")))
+
+ val df5 = spark.table("h2.test.address").filter($"email".startsWith("abc_'%"))
+ checkFiltersRemoved(df5)
+ checkPushedInfo(df5, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_'\\%%']")
Review Comment:
interesting, is this a valid SQL? how does the parser parse `EMAIL LIKE 'abc\\_'\\%%'`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400077892
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala:
##########
@@ -1262,6 +1275,85 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkAnswer(df17, Seq(Row(6, "jen", 12000, 1200, true)))
}
+ test("SPARK-38432: escape the quote, _ and % for DS V2 pushdown") {
+ val df1 = spark.table("h2.test.address").filter($"email".startsWith("abc_"))
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_%']")
+ checkAnswer(df1,
+ Seq(Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df2 = spark.table("h2.test.address").filter($"email".startsWith("abc%"))
+ checkFiltersRemoved(df2)
+ checkPushedInfo(df2, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%%']")
+ checkAnswer(df2, Seq(Row("abc%_def@gmail.com"), Row("abc%def@gmail.com")))
+
+ val df3 = spark.table("h2.test.address").filter($"email".startsWith("abc%_"))
+ checkFiltersRemoved(df3)
+ checkPushedInfo(df3, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\%\\_%']")
+ checkAnswer(df3, Seq(Row("abc%_def@gmail.com")))
+
+ val df4 = spark.table("h2.test.address").filter($"email".startsWith("abc_%"))
+ checkFiltersRemoved(df4)
+ checkPushedInfo(df4, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_\\%%']")
+ checkAnswer(df4, Seq(Row("abc_%def@gmail.com")))
+
+ val df5 = spark.table("h2.test.address").filter($"email".startsWith("abc_'%"))
+ checkFiltersRemoved(df5)
+ checkPushedInfo(df5, "PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\\_'\\%%']")
Review Comment:
I'm not sure. I guess it is different from each databases.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-38432][SQL][FOLLOWUP] Escape the _ and % for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1400114717
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -48,6 +48,29 @@
*/
public class V2ExpressionSQLBuilder {
+ protected String escapeSpecialChar(String str) {
+ StringBuilder builder = new StringBuilder();
+
+ for (char c : str.toCharArray()) {
+ switch (c) {
+ case '_':
+ builder.append("\\_");
+ break;
+ case '%':
+ builder.append("\\%");
+ break;
+ case '\'':
+ builder.append("\'");
Review Comment:
```suggestion
builder.append("\\\'");
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1403333706
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCV2Suite.scala:
##########
@@ -1262,6 +1275,94 @@ class JDBCV2Suite extends QueryTest with SharedSparkSession with ExplainSuiteHel
checkAnswer(df17, Seq(Row(6, "jen", 12000, 1200, true)))
}
+ test("SPARK-38432: escape the single quote, _ and % for DS V2 pushdown") {
+ val df1 = spark.table("h2.test.address").filter($"email".startsWith("abc_"))
+ checkFiltersRemoved(df1)
+ checkPushedInfo(df1, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_%' ESCAPE '\']")
+ checkAnswer(df1,
+ Seq(Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df2 = spark.table("h2.test.address").filter($"email".startsWith("abc%"))
+ checkFiltersRemoved(df2)
+ checkPushedInfo(df2, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\%%' ESCAPE '\']")
+ checkAnswer(df2, Seq(Row("abc%_def@gmail.com"), Row("abc%def@gmail.com")))
+
+ val df3 = spark.table("h2.test.address").filter($"email".startsWith("abc%_"))
+ checkFiltersRemoved(df3)
+ checkPushedInfo(df3, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\%\_%' ESCAPE '\']")
+ checkAnswer(df3, Seq(Row("abc%_def@gmail.com")))
+
+ val df4 = spark.table("h2.test.address").filter($"email".startsWith("abc_%"))
+ checkFiltersRemoved(df4)
+ checkPushedInfo(df4, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_\%%' ESCAPE '\']")
+ checkAnswer(df4, Seq(Row("abc_%def@gmail.com")))
+
+ val df5 = spark.table("h2.test.address").filter($"email".startsWith("abc_'%"))
+ checkFiltersRemoved(df5)
+ checkPushedInfo(df5,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE 'abc\_'\%%' ESCAPE '\']")
+ checkAnswer(df5, Seq(Row("abc_'%def@gmail.com")))
+
+ val df6 = spark.table("h2.test.address").filter($"email".endsWith("_def@gmail.com"))
+ checkFiltersRemoved(df6)
+ checkPushedInfo(df6,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_def@gmail.com' ESCAPE '\']")
+ checkAnswer(df6, Seq(Row("abc%_def@gmail.com"), Row("abc_def@gmail.com")))
+
+ val df7 = spark.table("h2.test.address").filter($"email".endsWith("%def@gmail.com"))
+ checkFiltersRemoved(df7)
+ checkPushedInfo(df7,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df7,
+ Seq(Row("abc%def@gmail.com"), Row("abc_%def@gmail.com"), Row("abc_'%def@gmail.com")))
+
+ val df8 = spark.table("h2.test.address").filter($"email".endsWith("%_def@gmail.com"))
+ checkFiltersRemoved(df8)
+ checkPushedInfo(df8,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\%\_def@gmail.com' ESCAPE '\']")
+ checkAnswer(df8, Seq(Row("abc%_def@gmail.com")))
+
+ val df9 = spark.table("h2.test.address").filter($"email".endsWith("_%def@gmail.com"))
+ checkFiltersRemoved(df9)
+ checkPushedInfo(df9,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df9, Seq(Row("abc_%def@gmail.com")))
+
+ val df10 = spark.table("h2.test.address").filter($"email".endsWith("_'%def@gmail.com"))
+ checkFiltersRemoved(df10)
+ checkPushedInfo(df10,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%\_'\%def@gmail.com' ESCAPE '\']")
+ checkAnswer(df10, Seq(Row("abc_'%def@gmail.com")))
+
+ val df11 = spark.table("h2.test.address").filter($"email".contains("c_d"))
+ checkFiltersRemoved(df11)
+ checkPushedInfo(df11, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_d%' ESCAPE '\']")
+ checkAnswer(df11, Seq(Row("abc_def@gmail.com")))
+
+ val df12 = spark.table("h2.test.address").filter($"email".contains("c%d"))
+ checkFiltersRemoved(df12)
+ checkPushedInfo(df12, raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\%d%' ESCAPE '\']")
+ checkAnswer(df12, Seq(Row("abc%def@gmail.com")))
+
+ val df13 = spark.table("h2.test.address").filter($"email".contains("c%_d"))
+ checkFiltersRemoved(df13)
+ checkPushedInfo(df13,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\%\_d%' ESCAPE '\']")
+ checkAnswer(df13, Seq(Row("abc%_def@gmail.com")))
+
+ val df14 = spark.table("h2.test.address").filter($"email".contains("c_%d"))
+ checkFiltersRemoved(df14)
+ checkPushedInfo(df14,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_\%d%' ESCAPE '\']")
+ checkAnswer(df14, Seq(Row("abc_%def@gmail.com")))
+
+ val df15 = spark.table("h2.test.address").filter($"email".contains("c_'%d"))
+ checkFiltersRemoved(df15)
+ checkPushedInfo(df15,
+ raw"PushedFilters: [EMAIL IS NOT NULL, EMAIL LIKE '%c\_'\%d%' ESCAPE '\']")
Review Comment:
I don't think this is right. When we generate the `LIKE` clause, we should call `escapeSql` to escape the `'` in the like pattern. Are you sure there is an extra step later to escape it? I can't find such code in the codebase.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46029][SQL] Escape the single quote, `_` and `%` for DS V2 pushdown [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43801:
URL: https://github.com/apache/spark/pull/43801#discussion_r1408579624
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/H2Dialect.scala:
##########
@@ -242,6 +242,14 @@ private[sql] object H2Dialect extends JdbcDialect {
}
class H2SQLBuilder extends JDBCSQLBuilder {
+ override def escapeSpecialCharsForLikePattern(str: String): String = {
+ str.map {
+ case '_' => "\\_"
+ case '%' => "\\%"
+ case c => c.toString
Review Comment:
H2 does not need to escape `\`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org