Skip to content

[SPARK-19555][SQL] Improve the performance of StringUtils.escapeLikeRegex method #16893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[SPARK-19555][SQL] Improve the performance of StringUtils.escapeLikeRegex method #16893

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bbcdeda
[SPARK-19555][SQL] Improve the performance of StringUtils.escapeLikeR…
lins05 Feb 11, 2017
e68eab0
Add back the comments.
lins05 Feb 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 23 additions & 15 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/StringUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.apache.spark.sql.catalyst.util

import java.lang.StringBuilder
import java.util.regex.{Pattern, PatternSyntaxException}

import org.apache.spark.unsafe.types.UTF8String
Expand All @@ -27,21 +28,28 @@ object StringUtils {
// replace the % with .*, match 0 or more times with any character
def escapeLikeRegex(v: String): String = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: while you are at it, can you also make the var names better ?

v -> input
c -> currentChar
prev -> previousChar

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could further simplify this by replacing previousChar with a boolean, nextCharacterIsEscaped (or inEscape).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'm wrong: we can't quite do that simplification because the old code has a subtle bug related to backslash-escaping. Due to the complexity and terseness the old implementation, it's a little non-obvious to spot that the case (prev, '\\') => "" has the effect of always ignoring backslash characters, so this method is incapable of producing a backslash in its output. This is a problem if the user wants to write a LIKE pattern to match backslashes then this is impossible with the current code.

It turns out that this is covered by #15398, which also implements performance improvements for this code, so I guess this PR and JIRA is redundant :(

I thought #15398 had been merged / fixed by now, but I guess not.

if (!v.isEmpty) {
"(?s)" + (' ' +: v.init).zip(v).flatMap {
case (prev, '\\') => ""
case ('\\', c) =>
c match {
case '_' => "_"
case '%' => "%"
case _ => Pattern.quote("\\" + c)
}
case (prev, c) =>
c match {
case '_' => "."
case '%' => ".*"
case _ => Pattern.quote(Character.toString(c))
}
}.mkString
val sb = new StringBuilder("(?s)")
var prev = ' '
for (c <- v) {
val out = (prev, c) match {
case (prev, '\\') => ""
case ('\\', c) =>
c match {
case '_' => "_"
case '%' => "%"
case _ => Pattern.quote("\\" + c)
}
case (prev, c) =>
c match {
case '_' => "."
case '%' => ".*"
case _ => Pattern.quote(Character.toString(c))
}
}
prev = c
sb.append(out)
}
sb.toString()
} else {
v
}
Expand Down