[SPARK-9152][SQL] Implement code generation for Like and RLike #7561

viirya · 2015-07-21T06:42:28Z

JIRA: https://issues.apache.org/jira/browse/SPARK-9152

This PR implements code generation for Like and RLike.

tarekbecker · 2015-07-21T06:51:59Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringOperations.scala

+        s"${patternClass} pattern = $patternClass.compile($literalRight);"
+      } else {
+        s"""
+          StringBuilder regex = new StringBuilder("(?s)");


I am not sure, if StringBuilderis imported. If not define somewhere val sb = classOf[StringBuilder].getName and use $sb

You shouldn't use regex. You can create a save variable name with ctx.freshName. (Same for all other variable names)

SparkQA · 2015-07-21T08:17:51Z

Test build #37923 has finished for PR 7561 at commit 69f0fb6.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-07-21T09:38:05Z

Test build #37937 has finished for PR 7561 at commit 6cffe3c.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-07-21T10:08:07Z

Test build #37940 has finished for PR 7561 at commit a0fb76e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-07-21T10:55:28Z

Test build #37945 has finished for PR 7561 at commit aea58e0.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2015-07-21T14:42:33Z

retest this please.

SparkQA · 2015-07-21T16:14:35Z

Test build #43 has finished for PR 7561 at commit aea58e0.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-07-21T16:26:40Z

Test build #37955 has finished for PR 7561 at commit aea58e0.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

rxin · 2015-07-22T02:17:39Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringOperations.scala

+    val pattern = ctx.freshName("pattern")
+
+    val literalRight: String = right match {
+      case x @ Literal(value: String, StringType) => escape(value)


rather than checking whether it is a string literal, i think you should check whether the expression is foldable, and and if yes, fold it.

rxin · 2015-07-22T02:33:14Z

cc @davies

davies · 2015-07-22T04:59:23Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringOperations.scala

+    val patternClass = classOf[Pattern].getName
+    val pattern = ctx.freshName("pattern")
+
+    val literalRight: String = right match {


Once we can cache the compiled pattern, this is not needed anymore.

SparkQA · 2015-07-22T07:13:42Z

Test build #38052 has finished for PR 7561 at commit 696d451.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2015-07-22T07:54:42Z

retest this please.

SparkQA · 2015-07-22T07:56:41Z

Test build #53 has finished for PR 7561 at commit 696d451.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-07-22T08:01:03Z

Test build #38056 has finished for PR 7561 at commit 696d451.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2015-07-22T08:05:57Z

Looks like Jenkins is not working now?

viirya · 2015-07-23T03:03:30Z

retest this please.

SparkQA · 2015-07-23T04:50:01Z

Test build #70 has finished for PR 7561 at commit 696d451.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-07-23T04:53:25Z

Test build #38153 has finished for PR 7561 at commit 696d451.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2015-07-24T06:44:25Z

ping @davies @rxin

rxin · 2015-07-27T07:25:24Z

cc @cloud-fan for review

cloud-fan · 2015-07-27T07:36:15Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringOperations.scala

+          $patternCode
+        }
+      }
+    """


You can use nullSafeCodeGen here.

Thanks. Updated.

SparkQA · 2015-07-27T14:59:23Z

Test build #38546 has finished for PR 7561 at commit 50df9a8.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

rxin · 2015-07-27T16:33:52Z

Jenkins, retest this please.

viirya · 2015-07-27T16:41:14Z

Same failure found in #7693. @liancheng is fixing it now. Currently retesting might also get failed.

viirya · 2015-07-28T02:22:16Z

retest this please.

SparkQA · 2015-07-28T03:56:23Z

Test build #128 has finished for PR 7561 at commit 50df9a8.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-07-28T03:58:16Z

Test build #38630 has finished for PR 7561 at commit 50df9a8.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2015-07-30T07:39:24Z

ping @rxin

davies · 2015-07-30T18:15:55Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringOperations.scala

+    val pattern = ctx.freshName("pattern")
+
+    nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
+      val patternCode =


nit: Do we need this?

davies · 2015-07-30T18:18:50Z

LGTM, merging this into master, the unneeded variable will be removed while merging.

rxin · 2015-07-30T18:19:48Z

@davies this one too - need unit tests that use NonFoldableLiteral

davies · 2015-07-30T18:25:23Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringOperations.scala

+
+    nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
+      val patternCode =
+        if (right.foldable) {


I think we should move this out of nullSafeCodeGen, or it will also eval the eval2.

if (right.foldable) { val rVal = right.eval() if (rVal != null) { } else { "boolean {ev.isNull} = true; xxx" } } else { nullSafeCodeGen(xxx) }

davies · 2015-07-30T18:26:53Z

Actually I didn't merge this, so go ahead.

rxin · 2015-07-31T04:30:31Z

@viirya can you update the unit test for rlike and like to use NonFoldableLiteral in addition to Literal?

viirya · 2015-07-31T04:34:58Z

@rxin ok.

rxin · 2015-07-31T04:41:25Z

LGTM

SparkQA · 2015-07-31T06:03:34Z

Test build #39152 has finished for PR 7561 at commit ccd1b43.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

rxin · 2015-07-31T06:06:16Z

Thanks - I've merged this.

SparkQA · 2015-07-31T06:15:03Z

Test build #39155 has finished for PR 7561 at commit fe5641b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Add code generation for Like and RLike.

69f0fb6

tarekbecker reviewed Jul 21, 2015
View reviewed changes

For comments.

6cffe3c

viirya added 3 commits July 21, 2015 16:24

For comments.

a0fb76e

Merge remote-tracking branch 'upstream/master' into like_rlike_codegen

46d946f

For comments.

aea58e0

rxin reviewed Jul 22, 2015
View reviewed changes

davies reviewed Jul 22, 2015
View reviewed changes

viirya added 2 commits July 22, 2015 15:01

Merge remote-tracking branch 'upstream/master' into like_rlike_codegen

48e5536

Check expression foldable.

696d451

cloud-fan reviewed Jul 27, 2015
View reviewed changes

viirya added 2 commits July 27, 2015 19:20

Merge remote-tracking branch 'upstream/master' into like_rlike_codegen

8092a68

Use nullSafeCodeGen.

50df9a8

davies reviewed Jul 30, 2015
View reviewed changes

viirya added 2 commits July 31, 2015 12:25

Merge remote-tracking branch 'upstream/master' into like_rlike_codegen

0086723

For comments.

ccd1b43

Add test for NonFoldableLiteral.

fe5641b

asfgit closed this in 0244170 Jul 31, 2015

viirya deleted the like_rlike_codegen branch December 27, 2023 18:18

[SPARK-9152][SQL] Implement code generation for Like and RLike #7561

[SPARK-9152][SQL] Implement code generation for Like and RLike #7561

Conversation

viirya commented Jul 21, 2015

tarekbecker Jul 21, 2015

Choose a reason for hiding this comment

SparkQA commented Jul 21, 2015

SparkQA commented Jul 21, 2015

SparkQA commented Jul 21, 2015

SparkQA commented Jul 21, 2015

viirya commented Jul 21, 2015

SparkQA commented Jul 21, 2015

SparkQA commented Jul 21, 2015

rxin Jul 22, 2015

Choose a reason for hiding this comment

rxin commented Jul 22, 2015

davies Jul 22, 2015

Choose a reason for hiding this comment

SparkQA commented Jul 22, 2015

viirya commented Jul 22, 2015

SparkQA commented Jul 22, 2015

SparkQA commented Jul 22, 2015

viirya commented Jul 22, 2015

viirya commented Jul 23, 2015

SparkQA commented Jul 23, 2015

SparkQA commented Jul 23, 2015

viirya commented Jul 24, 2015

rxin commented Jul 27, 2015

cloud-fan Jul 27, 2015

Choose a reason for hiding this comment

viirya Jul 27, 2015

Choose a reason for hiding this comment

SparkQA commented Jul 27, 2015

rxin commented Jul 27, 2015

viirya commented Jul 27, 2015

viirya commented Jul 28, 2015

SparkQA commented Jul 28, 2015

SparkQA commented Jul 28, 2015

viirya commented Jul 30, 2015

davies Jul 30, 2015

Choose a reason for hiding this comment

davies commented Jul 30, 2015

rxin commented Jul 30, 2015

davies Jul 30, 2015

Choose a reason for hiding this comment

davies commented Jul 30, 2015

rxin commented Jul 31, 2015

viirya commented Jul 31, 2015

rxin commented Jul 31, 2015

SparkQA commented Jul 31, 2015

rxin commented Jul 31, 2015

SparkQA commented Jul 31, 2015