[SPARK-4937][SQL] Adding optimization to simplify the And, Or condition in spark sql by scwf · Pull Request #3778 · apache/spark

scwf · 2014-12-23T13:27:56Z

Adding optimization to simplify the And/Or condition in spark sql.

There are two kinds of Optimization
1 Numeric condition optimization, such as:
a < 3 && a > 5 ---- False
a < 1 || a > 0 ---- True
a > 3 && a > 5 => a > 5
(a < 2 || b > 5) && a < 2 => a < 2

2 optimizing the some query from a cartesian product into equi-join, such as this sql (one of hive-testbench):

select
sum(l_extendedprice* (1 - l_discount)) as revenue
from
lineitem,
part
where
(
p_partkey = l_partkey
and p_brand = 'Brand#32'
and p_container in ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG')
and l_quantity >= 7 and l_quantity <= 7 + 10
and p_size between 1 and 5
and l_shipmode in ('AIR', 'AIR REG')
and l_shipinstruct = 'DELIVER IN PERSON'
)
or
(
p_partkey = l_partkey
and p_brand = 'Brand#35'
and p_container in ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK')
and l_quantity >= 15 and l_quantity <= 15 + 10
and p_size between 1 and 10
and l_shipmode in ('AIR', 'AIR REG')
and l_shipinstruct = 'DELIVER IN PERSON'
)
or
(
p_partkey = l_partkey
and p_brand = 'Brand#24'
and p_container in ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG')
and l_quantity >= 26 and l_quantity <= 26 + 10
and p_size between 1 and 15
and l_shipmode in ('AIR', 'AIR REG')
and l_shipinstruct = 'DELIVER IN PERSON'
)

It has a repeated expression in Or, so we can optimize it by (a && b) || (a && c) = a && (b || c)
Before optimization, this sql hang in my locally test, and the physical plan is:

After optimization, this sql run successfully in 20+ seconds, and its physical plan is:

This PR focus on the second optimization and some simple ones of the first. For complex Numeric condition optimization, I will make a follow up PR.

SparkQA · 2014-12-23T13:32:30Z

Test build #24738 has started for PR 3778 at commit 2f4a35d.

This patch merges cleanly.

SparkQA · 2014-12-23T13:33:21Z

Test build #24738 has finished for PR 3778 at commit 2f4a35d.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class CombinePredicate extends BinaryPredicate
- case class And(left: Expression, right: Expression) extends CombinePredicate
- case class Or(left: Expression, right: Expression) extends CombinePredicate
- case class SplitFragment(unit: Expression, remain: Option[Expression])
- implicit class CombinePredicateExtension(source: CombinePredicate)
- implicit class ExpressionCookies(expression: Expression)

AmplabJenkins · 2014-12-23T13:33:23Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24738/
Test FAILed.

SparkQA · 2014-12-23T13:42:28Z

Test build #24739 has started for PR 3778 at commit 6ec1aa1.

This patch merges cleanly.

SparkQA · 2014-12-23T13:53:05Z

Test build #24739 has finished for PR 3778 at commit 6ec1aa1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class CombinePredicate extends BinaryPredicate
- case class And(left: Expression, right: Expression) extends CombinePredicate
- case class Or(left: Expression, right: Expression) extends CombinePredicate
- case class SplitFragment(unit: Expression, remain: Option[Expression])
- implicit class CombinePredicateExtension(source: CombinePredicate)
- implicit class ExpressionCookies(expression: Expression)

AmplabJenkins · 2014-12-23T13:53:08Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24739/
Test FAILed.

liancheng · 2014-12-23T19:18:01Z

Hey @scwf, I'm kinda lost in all the predicates provided in the PR description... Would you mind to provide several simpler cases to illustrate what kind of optimizations this PR enables?

I guess essentially the second case is this:

(a && b) || (a && c) is turned into a && (b || c),
a happens to be a join key equality comparison, thus forms an equi-join
the cartesian product is replaced by a shuffle join

Is it correct?

scwf · 2014-12-24T03:03:01Z

Yes, correct, later i will give a detail description for this:)

scwf · 2014-12-24T05:45:52Z

change to WIP, todos:
1 fix test case
2 optimize and simplify the code

liancheng · 2014-12-24T06:06:36Z

@scwf These optimizations are useful, particularly the one that eliminates common predicates. Thanks for bringing them up! However, the implementation in this PR is really over complicated... I opened #3784 for a simpler version.

For the numeric comparisons, I think we can first cast all numeric comparisons to Double type and derive a Double range from each predicate and then eliminate unnecessary comparisons by comparing ranges. Double is preferred because Double.PositiveInfinity and Double.NegativeInfinity can be handy when dealing with predicates like a < 10 and a > 100.

SparkQA · 2014-12-24T07:02:28Z

Test build #24768 has started for PR 3778 at commit f1a487f.

This patch merges cleanly.

SparkQA · 2014-12-24T07:22:33Z

Test build #24773 has started for PR 3778 at commit 8c0316f.

This patch merges cleanly.

SparkQA · 2014-12-24T07:44:00Z

Test build #24768 has finished for PR 3778 at commit f1a487f.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class CombinePredicate extends BinaryPredicate
- case class And(left: Expression, right: Expression) extends CombinePredicate
- case class Or(left: Expression, right: Expression) extends CombinePredicate
- implicit class CombinePredicateExtension(source: CombinePredicate)
- implicit class ExpressionCookies(expression: Expression)

AmplabJenkins · 2014-12-24T07:44:03Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24768/
Test FAILed.

SparkQA · 2014-12-24T08:06:19Z

Test build #24773 has finished for PR 3778 at commit 8c0316f.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class CombinePredicate extends BinaryPredicate
- case class And(left: Expression, right: Expression) extends CombinePredicate
- case class Or(left: Expression, right: Expression) extends CombinePredicate
- implicit class CombinePredicateExtension(source: CombinePredicate)
- implicit class ExpressionCookies(expression: Expression)

AmplabJenkins · 2014-12-24T08:06:21Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24773/
Test FAILed.

SparkQA · 2014-12-25T03:07:34Z

Test build #24804 has started for PR 3778 at commit 8733027.

This patch merges cleanly.

SparkQA · 2014-12-25T04:17:10Z

Test build #24804 has finished for PR 3778 at commit 8733027.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class CombinePredicate extends BinaryPredicate
- case class And(left: Expression, right: Expression) extends CombinePredicate
- case class Or(left: Expression, right: Expression) extends CombinePredicate
- implicit class CombinePredicateExtension(source: CombinePredicate)
- implicit class ExpressionCookies(expression: Expression)

AmplabJenkins · 2014-12-25T04:17:13Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24804/
Test PASSed.

liancheng · 2014-12-25T16:28:56Z

Usually similar deeply nested if statements can be refactored into carefully organized pattern matches. Using pattern matching also frees you from helper methods like .isLess and isLessEquals, thus ExpressionCookies won't be necessary.

Get it, actually i have recognized this. but the case here really complex...:)
I am trying to refactory this

scwf · 2014-12-30T22:58:42Z

Hi, @marmbrus this is a more complete version than #3784 and @liancheng told me he will close #3784 in favor of this PR, but now you have merged #3784. So can you revert that one or Maybe should i change this with newly master branch?

marmbrus · 2014-12-30T23:30:04Z

Oh I see, sorry that was not clear from the discussion. I think it would be best to do this as an augmentation to what is already merged in.

scwf · 2014-12-30T23:37:17Z

Sure, updated!

SparkQA · 2014-12-30T23:37:30Z

Test build #24917 has started for PR 3778 at commit 9570211.

This patch merges cleanly.

SparkQA · 2014-12-31T00:50:27Z

Test build #24917 has finished for PR 3778 at commit 9570211.

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

AmplabJenkins · 2014-12-31T00:50:30Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24917/
Test PASSed.

scwf · 2015-01-10T23:25:12Z

@marmbrus, any comments here? i think this is ok to go

marmbrus · 2015-01-11T01:12:08Z

Nit: the indenting is off here.

marmbrus · 2015-01-11T01:23:20Z

This looks good to me. @liancheng have you looked this over too?

SparkQA · 2015-01-11T01:27:41Z

Test build #25368 has started for PR 3778 at commit 58bcbc2.

This patch merges cleanly.

SparkQA · 2015-01-11T02:32:46Z

Test build #25368 has finished for PR 3778 at commit 58bcbc2.

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

AmplabJenkins · 2015-01-11T02:32:49Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25368/
Test PASSed.

scwf · 2015-01-13T06:49:15Z

hey @liancheng, any other comments here?

liancheng · 2015-01-13T16:13:05Z

Sorry for the late reply, this LGTM, thanks!

scwf · 2015-01-14T01:04:20Z

Thanks, @marmbrus this should be ready to go.

marmbrus · 2015-01-16T22:01:37Z

Thanks! Merged to master.

rxin · 2015-01-17T05:33:42Z

I know this has been merged, but can you submit another PR to add comments explaining how this works? I'm afraid this block of code is beyond a normal engineer's capability to understand without spending hours staring at it. Thanks.

Sure, later i can do this

…nSimplification` Follow up of #3778 /cc rxin Author: scwf <wangfei1@huawei.com> Closes#4086 from scwf/commentforspark-4937 and squashes the following commits: aaf89f6 [scwf] code style issue 2d3406e [scwf] added comment for spark-4937

scwf changed the title ~~[SPARK-4938][SQL] Adding optimization to simplify the filter condition~~[SPARK-4937][SQL] Adding optimization to simplify the filter conditionDec 23, 2014

scwf changed the title ~~[SPARK-4937][SQL] Adding optimization to simplify the filter condition~~[WIP][SPARK-4937][SQL] Adding optimization to simplify the filter conditionDec 24, 2014

liancheng mentioned this pull request Dec 24, 2014
[SPARK-4937][SQL] Normalizes conjunctions and disjunctions to eliminate common predicates #3784
Closed

scwf force-pushed the filter1 branch from f1a487f to 8c0316fCompare December 24, 2014 07:18

scwf force-pushed the filter1 branch from 8c0316f to 8733027Compare December 25, 2014 03:04

scwf changed the title ~~[WIP][SPARK-4937][SQL] Adding optimization to simplify the filter condition~~[WIP][SPARK-4937][SQL] Adding optimization to simplify the And, Or condition in spark sqlDec 25, 2014

liancheng reviewed Dec 25, 2014
View reviewed changes

scwf changed the title ~~[WIP][SPARK-4937][SQL] Adding optimization to simplify the And, Or condition in spark sql~~[SPARK-4937][SQL] Adding optimization to simplify the And, Or condition in spark sqlDec 30, 2014

conflicts fix
9570211

marmbrus reviewed Jan 11, 2015
View reviewed changes

minor format fix
58bcbc2

asfgit closed this in ee1c1f3Jan 16, 2015

scwf deleted the filter1 branch January 16, 2015 22:03

rxin reviewed Jan 17, 2015
View reviewed changes

scwf mentioned this pull request Jan 17, 2015
[SPARK-4937][SQL] Comment for the newly optimization rules in BooleanSimplification#4086
Closed

Uh oh!

Conversation

scwf commented Dec 23, 2014

Uh oh!

SparkQA commented Dec 23, 2014

Uh oh!

SparkQA commented Dec 23, 2014

Uh oh!

AmplabJenkins commented Dec 23, 2014

Uh oh!

SparkQA commented Dec 23, 2014

Uh oh!

SparkQA commented Dec 23, 2014

Uh oh!

AmplabJenkins commented Dec 23, 2014

Uh oh!

liancheng commented Dec 23, 2014

Uh oh!

scwf commented Dec 24, 2014

Uh oh!

scwf commented Dec 24, 2014

Uh oh!

liancheng commented Dec 24, 2014

Uh oh!

SparkQA commented Dec 24, 2014

Uh oh!

SparkQA commented Dec 24, 2014

Uh oh!

SparkQA commented Dec 24, 2014

Uh oh!

AmplabJenkins commented Dec 24, 2014

Uh oh!

SparkQA commented Dec 24, 2014

Uh oh!

AmplabJenkins commented Dec 24, 2014

Uh oh!

SparkQA commented Dec 25, 2014

Uh oh!

SparkQA commented Dec 25, 2014

Uh oh!

AmplabJenkins commented Dec 25, 2014

Uh oh!

lianchengDec 25, 2014

Choose a reason for hiding this comment

Uh oh!

scwfDec 26, 2014

Choose a reason for hiding this comment

Uh oh!

scwf commented Dec 30, 2014

Uh oh!

marmbrus commented Dec 30, 2014

Uh oh!

scwf commented Dec 30, 2014

Uh oh!

SparkQA commented Dec 30, 2014

Uh oh!

SparkQA commented Dec 31, 2014

Uh oh!

AmplabJenkins commented Dec 31, 2014

Uh oh!

scwf commented Jan 10, 2015

Uh oh!

marmbrusJan 11, 2015

Choose a reason for hiding this comment

Uh oh!

marmbrus commented Jan 11, 2015

Uh oh!

SparkQA commented Jan 11, 2015

Uh oh!

SparkQA commented Jan 11, 2015

Uh oh!

AmplabJenkins commented Jan 11, 2015

Uh oh!

scwf commented Jan 13, 2015

Uh oh!

liancheng commented Jan 13, 2015

Uh oh!

scwf commented Jan 14, 2015

Uh oh!

marmbrus commented Jan 16, 2015