Skip to content
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.

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

[SPARK-9403][SQL] Add codegen support in In and InSet #7893

Closed
wants to merge 10 commits into from

Conversation

viirya
Copy link
Member

@viirya viirya commented Aug 3, 2015

This continues @tarekauel's work in #7778.

val value = dataGen.apply()
value match {
case d: Double if d.isNaN => 0.0d
case f: Float if f.isNaN => 0.0f
Copy link
Member Author

Choose a reason for hiding this comment

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

Double.NaN == Double.NaN is false. So here we don't use it.

@SparkQA
Copy link

SparkQA commented Aug 3, 2015

Test build #39530 has finished for PR 7893 at commit 224f18e.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class In(value: Expression, list: Seq[Expression]) extends Predicate
    • case class InSet(child: Expression, hset: Set[Any]) extends UnaryExpression with Predicate

@SparkQA
Copy link

SparkQA commented Aug 3, 2015

Test build #39543 has finished for PR 7893 at commit 4610eff.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class In(value: Expression, list: Seq[Expression]) extends Predicate
    • case class InSet(child: Expression, hset: Set[Any]) extends UnaryExpression with Predicate

@SparkQA
Copy link

SparkQA commented Aug 3, 2015

Test build #39552 has finished for PR 7893 at commit 446bbcd.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class In(value: Expression, list: Seq[Expression]) extends Predicate
    • case class InSet(child: Expression, hset: Set[Any]) extends UnaryExpression with Predicate

@rxin
Copy link
Contributor

rxin commented Aug 3, 2015

cc @davies for review

@@ -97,7 +101,7 @@ case class Not(child: Expression)
/**
* Evaluates to `true` if `list` contains `value`.
*/
case class In(value: Expression, list: Seq[Expression]) extends Predicate with CodegenFallback {
case class In(value: Expression, list: Seq[Expression]) extends Predicate {
Copy link
Contributor

Choose a reason for hiding this comment

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

Existing: Should we check that value and element of list have the same data type?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it ok to add a check in CheckAnalysis for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes,

Copy link
Contributor

Choose a reason for hiding this comment

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

@SparkQA
Copy link

SparkQA commented Aug 4, 2015

Test build #39701 has finished for PR 7893 at commit cf4bf41.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class In(value: Expression, list: Seq[Expression]) extends Predicate
    • case class InSet(child: Expression, hset: Set[Any]) extends UnaryExpression with Predicate

@viirya
Copy link
Member Author

viirya commented Aug 4, 2015

retest this please.

${ev.primitive} = true;
}
}
""").foldLeft("")((a, b) => a + "\n" + b)
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually use mkString("\n")

@davies
Copy link
Contributor

davies commented Aug 4, 2015

LGTM, except some minor comments.

@SparkQA
Copy link

SparkQA commented Aug 4, 2015

Test build #39713 has finished for PR 7893 at commit cf4bf41.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class In(value: Expression, list: Seq[Expression]) extends Predicate
    • case class InSet(child: Expression, hset: Set[Any]) extends UnaryExpression with Predicate

@SparkQA
Copy link

SparkQA commented Aug 4, 2015

Test build #208 has finished for PR 7893 at commit cf4bf41.

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

@viirya
Copy link
Member Author

viirya commented Aug 5, 2015

retest this please.

@SparkQA
Copy link

SparkQA commented Aug 5, 2015

Test build #225 has finished for PR 7893 at commit 81ff97b.

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

@viirya
Copy link
Member Author

viirya commented Aug 5, 2015

Jenkins looks like unstable...

@viirya
Copy link
Member Author

viirya commented Aug 5, 2015

retest this please.

@SparkQA
Copy link

SparkQA commented Aug 5, 2015

Test build #39846 has finished for PR 7893 at commit 81ff97b.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class In(value: Expression, list: Seq[Expression]) extends Predicate
    • case class InSet(child: Expression, hset: Set[Any]) extends UnaryExpression with Predicate

@SparkQA
Copy link

SparkQA commented Aug 5, 2015

Test build #1350 has finished for PR 7893 at commit 81ff97b.

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

@SparkQA
Copy link

SparkQA commented Aug 5, 2015

Test build #228 has finished for PR 7893 at commit 81ff97b.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class In(value: Expression, list: Seq[Expression]) extends Predicate
    • case class InSet(child: Expression, hset: Set[Any]) extends UnaryExpression with Predicate

@SparkQA
Copy link

SparkQA commented Aug 5, 2015

Test build #39833 has finished for PR 7893 at commit 81ff97b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class In(value: Expression, list: Seq[Expression]) extends Predicate
    • case class InSet(child: Expression, hset: Set[Any]) extends UnaryExpression with Predicate

@davies
Copy link
Contributor

davies commented Aug 5, 2015

LGTM, merging this into master and 1.5 branch, thanks!

asfgit pushed a commit that referenced this pull request Aug 5, 2015
This continues tarekauel's work in #7778.

Author: Liang-Chi Hsieh <viirya@appier.com>
Author: Tarek Auel <tarek.auel@googlemail.com>

Closes #7893 from viirya/codegen_in and squashes the following commits:

81ff97b [Liang-Chi Hsieh] For comments.
47761c6 [Liang-Chi Hsieh] Merge remote-tracking branch 'upstream/master' into codegen_in
cf4bf41 [Liang-Chi Hsieh] For comments.
f532b3c [Liang-Chi Hsieh] Merge remote-tracking branch 'upstream/master' into codegen_in
446bbcd [Liang-Chi Hsieh] Fix bug.
b3d0ab4 [Liang-Chi Hsieh] Merge remote-tracking branch 'upstream/master' into codegen_in
4610eff [Liang-Chi Hsieh] Relax the types of references and update optimizer test.
224f18e [Liang-Chi Hsieh] Beef up the test cases for In and InSet to include all primitive data types.
86dc8aa [Liang-Chi Hsieh] Only convert In to InSet when the number of items in set is more than the threshold.
b7ded7e [Tarek Auel] [SPARK-9403][SQL] codeGen in / inSet

(cherry picked from commit e1e0587)
Signed-off-by: Davies Liu <davies.liu@gmail.com>
@asfgit asfgit closed this in e1e0587 Aug 5, 2015
asfgit pushed a commit that referenced this pull request Aug 5, 2015
@viirya viirya deleted the codegen_in branch December 27, 2023 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants