viirya commented on code in PR #59879:
URL: https://github.com/apache/spark/pull/59879#discussion_r4405406336
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/RegexpExpressionsSuite.scala:
##########
@@ -735,4 +735,28 @@ class RegexpExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
assert(extractAllCopy ne extractAll,
"freshCopyIfContainsStatefulExpression should return a new instance for
RegExpExtractAll")
}
+
+ test("SPARK-60632: LIKE and RLIKE expensive heuristic looks at the pattern")
{
Review Comment:
Added: an RLIKE or LIKE over a `ScalaUDF` subject with a cheap pattern is
expensive. `FilterPushdownSuite` now also pushes `^a.*b` and an ILIKE in the
form the optimizer sees it, `Like(Lower(e), Lower('%Magic%'))`, and does not
push `.*a.*b` or `%a%b%c`.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/regexpExpressions.scala:
##########
@@ -84,33 +84,32 @@ abstract class StringRegexExpression extends
BinaryExpression
}
}
- override def expensive: Boolean = hasExpensiveChild || _expensiveRegex
-
- // Heuristic, not designed to be perfect. Look for things likely to have
- // back tracking.
- private val detectExpensiveRegexPattern = Pattern.compile("\\+\\*\\{")
-
- private lazy val _expensiveRegex = {
- // A quick heuristic for expensive a pattern is.
- left match {
- case StringLiteral(str) =>
- // If we have a clear start limited back tracking required.
- if (str.startsWith("^") || str.startsWith("\\b")) {
- false
- } else if (detectExpensiveRegexPattern.matcher(str).matches()) {
- // Greedy matching can be tricky.
- true
- } else {
- // Default to pushdown for now.
- false
- }
- case _ =>
- true // per row regex compilation.
- }
+ override def expensive: Boolean = hasExpensiveChild || _expensivePattern
+
+ // Heuristic, not designed to be perfect. A pattern that is not foldable is
compiled for every
+ // row; a foldable one is left to `isExpensivePattern`.
+ private lazy val _expensivePattern = if (right.foldable) {
+ val str = right.eval()
+ str != null && isExpensivePattern(str.toString)
+ } else {
+ true
}
+
+ /** Whether matching against the constant pattern `str` is likely to be
expensive. */
+ protected def isExpensivePattern(str: String): Boolean
}
private[catalyst] object StringRegexExpression {
+ // Quantifiers, which are what make a regex backtrack.
+ private val quantifierPattern = Pattern.compile("[+*{]")
+
+ /**
+ * Heuristic for whether a regex is likely to backtrack: one with a clear
start limits the
+ * backtracking, otherwise any quantifier may need it.
+ */
+ def isExpensiveRegex(regex: String): Boolean =
Review Comment:
The `Pattern` is gone. The counting has to skip escapes and character
classes, so it is a loop rather than a one-liner; it sits in the companion
object next to the LIKE counterpart. `Like` now uses the evaluated pattern too.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/FilterPushdownSuite.scala:
##########
@@ -61,6 +61,10 @@ class FilterPushdownSuite extends PlanTest {
val testStringRelation = LocalRelation(attrA, attrB, attrE)
+ // Stands in for an expensive expression: a regex whose pattern is not a
constant is compiled
+ // for every row.
+ private def expensiveRLike(tag: String): Expression =
$"e".rlike(Concat(Seq($"e", Literal(tag))))
Review Comment:
Moved after the relation and predicate definitions.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
