[GitHub] [spark] gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax
gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax URL: https://github.com/apache/spark/pull/25001#discussion_r354123985 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/regexpExpressions.scala ## @@ -83,16 +83,15 @@ abstract class StringRegexExpression extends BinaryExpression % matches zero or more characters in the input (similar to .* in posix regular expressions) - The escape character is '\'. If an escape character precedes a special symbol or another - escape character, the following character is matched literally. It is invalid to escape - any other character. - Since Spark 2.0, string literals are unescaped in our SQL parser. For example, in order to match "\abc", the pattern should be "\\abc". When SQL config 'spark.sql.parser.escapedStringLiterals' is enabled, it fallbacks to Spark 1.6 behavior regarding string literal parsing. For example, if the config is enabled, the pattern to match "\abc" should be "\abc". + * escape - an optional string added since Spark 3.0. The default escape character is the '\'. Review comment: Please update the comment as well. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax
gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL]
Support LIKE ... ESCAPE syntax
URL: https://github.com/apache/spark/pull/25001#discussion_r354124082
##
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
##
@@ -484,7 +484,7 @@ object LikeSimplification extends Rule[LogicalPlan] {
private val equalTo = "([^_%]*)".r
def apply(plan: LogicalPlan): LogicalPlan = plan transformAllExpressions {
-case Like(input, Literal(pattern, StringType)) =>
+case Like(input, Literal(pattern, StringType), opt) =>
Review comment:
opt => escapeChar
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax
gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL]
Support LIKE ... ESCAPE syntax
URL: https://github.com/apache/spark/pull/25001#discussion_r353893413
##
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala
##
@@ -98,7 +98,8 @@ package object dsl {
case _ => In(expr, list)
}
-def like(other: Expression): Expression = Like(expr, other)
+def like(other: Expression, escapeCharOpt: Option[Char] = None):
Expression =
Review comment:
Please consider changing this to
```
escapeChar: Char = '\\'
```
as well.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax
gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL]
Support LIKE ... ESCAPE syntax
URL: https://github.com/apache/spark/pull/25001#discussion_r353633377
##
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/regexpExpressions.scala
##
@@ -104,19 +103,24 @@ abstract class StringRegexExpression extends
BinaryExpression
spark.sql.parser.escapedStringLiterals false
> SELECT '%SystemDrive%\\Users\\John' _FUNC_ '\%SystemDrive\%Users%';
true
+ > SELECT '%SystemDrive%/Users/John' _FUNC_ '/%SystemDrive/%//Users%'
ESCAPE '/';
+ true
""",
note = """
Use RLIKE to match with standard regular expressions.
""",
since = "1.0.0")
// scalastyle:on line.contains.tab
-case class Like(left: Expression, right: Expression) extends
StringRegexExpression {
+case class Like(left: Expression, right: Expression, escapeCharOpt:
Option[Char] = None)
Review comment:
If the last parameter is '\' , then we can just ignore it in the `toString`
method.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax
gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL]
Support LIKE ... ESCAPE syntax
URL: https://github.com/apache/spark/pull/25001#discussion_r353361419
##
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/regexpExpressions.scala
##
@@ -104,19 +103,24 @@ abstract class StringRegexExpression extends
BinaryExpression
spark.sql.parser.escapedStringLiterals false
> SELECT '%SystemDrive%\\Users\\John' _FUNC_ '\%SystemDrive\%Users%';
true
+ > SELECT '%SystemDrive%/Users/John' _FUNC_ '/%SystemDrive/%//Users%'
ESCAPE '/';
+ true
""",
note = """
Use RLIKE to match with standard regular expressions.
""",
since = "1.0.0")
// scalastyle:on line.contains.tab
-case class Like(left: Expression, right: Expression) extends
StringRegexExpression {
+case class Like(left: Expression, right: Expression, escapeCharOpt:
Option[Char] = None)
Review comment:
How about just
```
escapeChar: Char = '\\'
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax
gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL]
Support LIKE ... ESCAPE syntax
URL: https://github.com/apache/spark/pull/25001#discussion_r353361419
##
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/regexpExpressions.scala
##
@@ -104,19 +103,24 @@ abstract class StringRegexExpression extends
BinaryExpression
spark.sql.parser.escapedStringLiterals false
> SELECT '%SystemDrive%\\Users\\John' _FUNC_ '\%SystemDrive\%Users%';
true
+ > SELECT '%SystemDrive%/Users/John' _FUNC_ '/%SystemDrive/%//Users%'
ESCAPE '/';
+ true
""",
note = """
Use RLIKE to match with standard regular expressions.
""",
since = "1.0.0")
// scalastyle:on line.contains.tab
-case class Like(left: Expression, right: Expression) extends
StringRegexExpression {
+case class Like(left: Expression, right: Expression, escapeCharOpt:
Option[Char] = None)
Review comment:
How about just change the last parameter as
```
escapeChar: Char = '\\'
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax
gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL]
Support LIKE ... ESCAPE syntax
URL: https://github.com/apache/spark/pull/25001#discussion_r353594929
##
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/regexpExpressions.scala
##
@@ -104,19 +103,24 @@ abstract class StringRegexExpression extends
BinaryExpression
spark.sql.parser.escapedStringLiterals false
> SELECT '%SystemDrive%\\Users\\John' _FUNC_ '\%SystemDrive\%Users%';
true
+ > SELECT '%SystemDrive%/Users/John' _FUNC_ '/%SystemDrive/%//Users%'
ESCAPE '/';
+ true
""",
note = """
Use RLIKE to match with standard regular expressions.
""",
since = "1.0.0")
// scalastyle:on line.contains.tab
-case class Like(left: Expression, right: Expression) extends
StringRegexExpression {
+case class Like(left: Expression, right: Expression, escapeCharOpt:
Option[Char] = None)
Review comment:
We can show `escapeChar` in `toString` if it is not the default value. It is
quite trivial for the method `toString`.
But changing the data type of this parameter can simplify the rest of the
code changes.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax
gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL]
Support LIKE ... ESCAPE syntax
URL: https://github.com/apache/spark/pull/25001#discussion_r353594929
##
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/regexpExpressions.scala
##
@@ -104,19 +103,24 @@ abstract class StringRegexExpression extends
BinaryExpression
spark.sql.parser.escapedStringLiterals false
> SELECT '%SystemDrive%\\Users\\John' _FUNC_ '\%SystemDrive\%Users%';
true
+ > SELECT '%SystemDrive%/Users/John' _FUNC_ '/%SystemDrive/%//Users%'
ESCAPE '/';
+ true
""",
note = """
Use RLIKE to match with standard regular expressions.
""",
since = "1.0.0")
// scalastyle:on line.contains.tab
-case class Like(left: Expression, right: Expression) extends
StringRegexExpression {
+case class Like(left: Expression, right: Expression, escapeCharOpt:
Option[Char] = None)
Review comment:
We can show `escapeChar` if it is not the default value. It is quite trivial
for the method `toString`.
But changing the data type of this parameter can simplify the rest of the
code changes.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax
gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL]
Support LIKE ... ESCAPE syntax
URL: https://github.com/apache/spark/pull/25001#discussion_r353361419
##
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/regexpExpressions.scala
##
@@ -104,19 +103,24 @@ abstract class StringRegexExpression extends
BinaryExpression
spark.sql.parser.escapedStringLiterals false
> SELECT '%SystemDrive%\\Users\\John' _FUNC_ '\%SystemDrive\%Users%';
true
+ > SELECT '%SystemDrive%/Users/John' _FUNC_ '/%SystemDrive/%//Users%'
ESCAPE '/';
+ true
""",
note = """
Use RLIKE to match with standard regular expressions.
""",
since = "1.0.0")
// scalastyle:on line.contains.tab
-case class Like(left: Expression, right: Expression) extends
StringRegexExpression {
+case class Like(left: Expression, right: Expression, escapeCharOpt:
Option[Char] = None)
Review comment:
How about just
```
escapeCharOpt: Char = '\\'
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax
gengliangwang commented on a change in pull request #25001: [SPARK-28083][SQL] Support LIKE ... ESCAPE syntax URL: https://github.com/apache/spark/pull/25001#discussion_r353358167 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/regexpExpressions.scala ## @@ -83,16 +83,15 @@ abstract class StringRegexExpression extends BinaryExpression % matches zero or more characters in the input (similar to .* in posix regular expressions) - The escape character is '\'. If an escape character precedes a special symbol or another - escape character, the following character is matched literally. It is invalid to escape - any other character. - Since Spark 2.0, string literals are unescaped in our SQL parser. For example, in order to match "\abc", the pattern should be "\\abc". When SQL config 'spark.sql.parser.escapedStringLiterals' is enabled, it fallbacks to Spark 1.6 behavior regarding string literal parsing. For example, if the config is enabled, the pattern to match "\abc" should be "\abc". + * escape - a optional string added since Spark 3.0. The default escape character is the '\'. Review comment: Nit: a => an 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
