You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by saucam <gi...@git.apache.org> on 2015/04/25 15:04:00 UTC
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
GitHub user saucam opened a pull request:
https://github.com/apache/spark/pull/5700
[SPARK-7142][SQL]: Minor enhancement to BooleanSimplification Optimizer rule
Use these in the optimizer as well:
A and (not(A) or B) => A and B
not(A and B) => not(A) or not(B)
not(A or B) => not(A) and not(B)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/saucam/spark bool_simp
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/5700.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #5700
----
commit 3eb813e66281b53ca029faa7928cc6c50a69b509
Author: Yash Datta <ya...@guavus.com>
Date: 2015-04-25T12:45:48Z
SPARK-7142: Minor enhancement to BooleanSimplification Optimizer rule, using these rules:
A and (not(A) or B) => A and B
not(A and B) => not(A) or not(B)
not(A or B) => not(A) and not(B)
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139168340
Merged build triggered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-96220491
[Test build #30953 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30953/consoleFull) for PR 5700 at commit [`9e1f8dd`](https://github.com/apache/spark/commit/9e1f8ddb9104cee4cc37c19c8c075ad312a1d6b7).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `final class IDF extends Estimator[IDFModel] with IDFBase `
* This patch **adds the following new dependencies:**
* `tachyon-0.6.4.jar`
* `tachyon-client-0.6.4.jar`
* This patch **removes the following dependencies:**
* `tachyon-0.5.0.jar`
* `tachyon-client-0.5.0.jar`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139168359
Merged build started.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139166614
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139142427
[Test build #42249 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42249/consoleFull) for PR 5700 at commit [`de2dbdc`](https://github.com/apache/spark/commit/de2dbdcf889add5602496a34bc796ef12242fdb0).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139166618
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42243/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139144960
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42249/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139131186
Merged build started.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-96200802
[Test build #30953 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30953/consoleFull) for PR 5700 at commit [`9e1f8dd`](https://github.com/apache/spark/commit/9e1f8ddb9104cee4cc37c19c8c075ad312a1d6b7).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by saucam <gi...@git.apache.org>.
Github user saucam commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139327218
thanks @marmbrus :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139140571
Merged build started.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/5700
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by saucam <gi...@git.apache.org>.
Github user saucam commented on a diff in the pull request:
https://github.com/apache/spark/pull/5700#discussion_r39256778
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
@@ -413,6 +418,10 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper {
case LessThan(l, r) => GreaterThanOrEqual(l, r)
// not(l <= r) => l > r
case LessThanOrEqual(l, r) => GreaterThan(l, r)
+ // not(l || r) => not(l) && not(r)
+ case Or(l, r) => And(Not(l), Not(r))
+ // not(l && r) => not(l) or not(r)
+ case And(l, r) => Or(Not(l), Not(r))
--- End diff --
@cloud-fan could you please explain a bit more when and how converting to "And" may not be an optimization ? I was wondering would it actually result in any kind of performance hit ? Also could you tell how #8200 is more reasonable ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/5700#discussion_r29110184
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
@@ -413,6 +418,10 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper {
case LessThan(l, r) => GreaterThanOrEqual(l, r)
// not(l <= r) => l > r
case LessThanOrEqual(l, r) => GreaterThan(l, r)
+ // not(l || r) => not(l) && not(r)
+ case Or(l, r) => And(Not(l), Not(r))
+ // not(l && r) => not(l) or not(r)
+ case And(l, r) => Or(Not(l), Not(r))
--- End diff --
How this 2 rules optimize execution?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by saucam <gi...@git.apache.org>.
Github user saucam commented on a diff in the pull request:
https://github.com/apache/spark/pull/5700#discussion_r29121549
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
@@ -413,6 +418,10 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper {
case LessThan(l, r) => GreaterThanOrEqual(l, r)
// not(l <= r) => l > r
case LessThanOrEqual(l, r) => GreaterThan(l, r)
+ // not(l || r) => not(l) && not(r)
+ case Or(l, r) => And(Not(l), Not(r))
+ // not(l && r) => not(l) or not(r)
+ case And(l, r) => Or(Not(l), Not(r))
--- End diff --
This is inside a case match :
```
case not @ Not(exp) => exp match {
....
....
case Or(l, r) => And(Not(l), Not(r))
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139168731
[Test build #42257 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42257/consoleFull) for PR 5700 at commit [`2dbfba6`](https://github.com/apache/spark/commit/2dbfba691283457f0367271d8348951123b8229d).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139319966
Thanks, merging to master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/5700#discussion_r39232023
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
@@ -434,6 +434,11 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper {
case (_, Literal(false, BooleanType)) => Literal(false)
// a && a => a
case (l, r) if l fastEquals r => l
+ // a && (not(a) || b) => a && b
+ case (l, Or(l1, r)) if (Not(l) fastEquals l1) => And(l, r)
+ case (l, Or(r, l1)) if (Not(l) fastEquals l1) => And(l, r)
+ case (Or(l, l1), r) if (l1 fastEquals Not(r)) => And(l, r)
+ case (Or(l1, l), r) if (l1 fastEquals Not(r)) => And(l, r)
--- End diff --
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-137564607
Thanks for working on this! Can you please add test cases for these optimizations?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139166496
[Test build #42243 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42243/console) for PR 5700 at commit [`7ada093`](https://github.com/apache/spark/commit/7ada093214359973f12284e3fc368c8e36bb1b17).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139144924
[Test build #42249 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42249/console) for PR 5700 at commit [`de2dbdc`](https://github.com/apache/spark/commit/de2dbdcf889add5602496a34bc796ef12242fdb0).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/5700#discussion_r29121291
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
@@ -413,6 +418,10 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper {
case LessThan(l, r) => GreaterThanOrEqual(l, r)
// not(l <= r) => l > r
case LessThanOrEqual(l, r) => GreaterThan(l, r)
+ // not(l || r) => not(l) && not(r)
+ case Or(l, r) => And(Not(l), Not(r))
+ // not(l && r) => not(l) or not(r)
+ case And(l, r) => Or(Not(l), Not(r))
--- End diff --
`case Not(Or(l, r))`? Seems you miss the `Not`...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by saucam <gi...@git.apache.org>.
Github user saucam commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139138411
added test cases
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139199120
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42257/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/5700#discussion_r39231942
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
@@ -413,6 +418,10 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper {
case LessThan(l, r) => GreaterThanOrEqual(l, r)
// not(l <= r) => l > r
case LessThanOrEqual(l, r) => GreaterThan(l, r)
+ // not(l || r) => not(l) && not(r)
+ case Or(l, r) => And(Not(l), Not(r))
+ // not(l && r) => not(l) or not(r)
+ case And(l, r) => Or(Not(l), Not(r))
--- End diff --
Changing `Not(And(a, b))` to `And(Not(a), Not(b))` may not always be an optimization. It do helps to push down filters for datasource, but I think https://github.com/apache/spark/pull/8200 is more reasonable since it only do this optimization in datasource part.
cc @marmbrus
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139131574
[Test build #42243 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42243/consoleFull) for PR 5700 at commit [`7ada093`](https://github.com/apache/spark/commit/7ada093214359973f12284e3fc368c8e36bb1b17).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by saucam <gi...@git.apache.org>.
Github user saucam commented on a diff in the pull request:
https://github.com/apache/spark/pull/5700#discussion_r29110517
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
@@ -413,6 +418,10 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper {
case LessThan(l, r) => GreaterThanOrEqual(l, r)
// not(l <= r) => l > r
case LessThanOrEqual(l, r) => GreaterThan(l, r)
+ // not(l || r) => not(l) && not(r)
+ case Or(l, r) => And(Not(l), Not(r))
+ // not(l && r) => not(l) or not(r)
+ case And(l, r) => Or(Not(l), Not(r))
--- End diff --
So for example the filter is not(Or(left, r)) , where r might be some filter on a partitioned column like part>=12 , in the present case this filter cannot be pushed down, since while evaluating we will encounter reference of partitioned column, whereas if this rule is applied we get And(not(l), part<12) and then not(l) might be pushed down since now splitting into conjunctive predicates is possible.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139140355
Merged build triggered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139199118
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139144952
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/5700#discussion_r39257887
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
@@ -413,6 +418,10 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper {
case LessThan(l, r) => GreaterThanOrEqual(l, r)
// not(l <= r) => l > r
case LessThanOrEqual(l, r) => GreaterThan(l, r)
+ // not(l || r) => not(l) && not(r)
+ case Or(l, r) => And(Not(l), Not(r))
+ // not(l && r) => not(l) or not(r)
+ case And(l, r) => Or(Not(l), Not(r))
--- End diff --
At first I thought this optimization should be only used in datasource as it may cause performance regression for normal cases(see these [comments](https://github.com/apache/spark/pull/8200#discussion-diff-37154582)). However after an offline discussion with @marmbrus , this optimization is pretty standard and generally good, so let's keep it in `Optimizer` :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/5700#discussion_r39231425
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
@@ -434,6 +434,11 @@ object BooleanSimplification extends Rule[LogicalPlan] with PredicateHelper {
case (_, Literal(false, BooleanType)) => Literal(false)
// a && a => a
case (l, r) if l fastEquals r => l
+ // a && (not(a) || b) => a && b
+ case (l, Or(l1, r)) if (Not(l) fastEquals l1) => And(l, r)
+ case (l, Or(r, l1)) if (Not(l) fastEquals l1) => And(l, r)
+ case (Or(l, l1), r) if (l1 fastEquals Not(r)) => And(l, r)
+ case (Or(l1, l), r) if (l1 fastEquals Not(r)) => And(l, r)
--- End diff --
Do we need `fastEquals` here? `Not(l)` and `l1` will never be a same reference and we always fallback to normal equality check. I think just here `==` here is more reasonable.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139131166
Merged build triggered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-7142][SQL]: Minor enhancement to Boolea...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5700#issuecomment-139199009
[Test build #42257 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/42257/console) for PR 5700 at commit [`2dbfba6`](https://github.com/apache/spark/commit/2dbfba691283457f0367271d8348951123b8229d).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org