You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gengliangwang <gi...@git.apache.org> on 2018/10/10 09:57:45 UTC

[GitHub] spark pull request #22687: [SPARK-25702][SQL] Push down filters with `Not` o...

GitHub user gengliangwang opened a pull request:

    https://github.com/apache/spark/pull/22687

    [SPARK-25702][SQL] Push down filters with `Not` operator in Parquet

    ## What changes were proposed in this pull request?
    
    Currently, in ParquetFilters, predicates inside `Not` operator are considered as unable to perform partial push down.
    However, the following cases is still possible for push down:
    1. `Not(Or(left, right))` can be conversed as `And(Not(left), Not(right))`
    2. `Not(Not(pred))` can be conversed as `pred`
    
    Both cases should be quite trivial, since the `Not` operator should be pushed down by optimization rule `BooleanSimplification` already.
    But I think it should be good to handle such cases in Parquet data source module as well.
    
    ## How was this patch tested?
    
    New unit test.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/gengliangwang/spark parquetNotFilters

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/22687.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 #22687
    
----
commit 0f43db656c3567ab7f8b711c8f0b27b16caa4bf7
Author: Gengliang Wang <ge...@...>
Date:   2018-10-10T09:40:45Z

    push down more parquet filters

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #22687: [SPARK-25702][SQL] Push down filters with `Not` o...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/22687#discussion_r224021798
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetFilters.scala ---
    @@ -534,6 +534,13 @@ private[parquet] class ParquetFilters(
                 createFilterHelper(nameToParquetField, rhs, canPartialPushDownConjuncts = false)
             } yield FilterApi.or(lhsFilter, rhsFilter)
     
    +      case sources.Not(sources.Or(lhs, rhs)) if canPartialPushDownConjuncts =>
    +        createFilterHelper(nameToParquetField,
    +          sources.And(sources.Not(lhs), sources.Not(rhs)), canPartialPushDownConjuncts = true)
    +
    +      case sources.Not(sources.Not(pred)) if canPartialPushDownConjuncts =>
    --- End diff --
    
    hm, is this actually reachable?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #22687: [SPARK-25702][SQL] Push down filters with `Not` operator...

Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on the issue:

    https://github.com/apache/spark/pull/22687
  
    It's OK. Close this one.
    Thanks for reviewing @viirya @HyukjinKwon 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #22687: [SPARK-25702][SQL] Push down filters with `Not` operator...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22687
  
    **[Test build #97193 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97193/testReport)** for PR 22687 at commit [`0f43db6`](https://github.com/apache/spark/commit/0f43db656c3567ab7f8b711c8f0b27b16caa4bf7).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #22687: [SPARK-25702][SQL] Push down filters with `Not` operator...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/22687
  
    I prefer not to add code that will not run. Let's see others options too.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #22687: [SPARK-25702][SQL] Push down filters with `Not` operator...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22687
  
    **[Test build #97193 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97193/testReport)** for PR 22687 at commit [`0f43db6`](https://github.com/apache/spark/commit/0f43db656c3567ab7f8b711c8f0b27b16caa4bf7).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #22687: [SPARK-25702][SQL] Push down filters with `Not` operator...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22687
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #22687: [SPARK-25702][SQL] Push down filters with `Not` operator...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22687
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3847/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #22687: [SPARK-25702][SQL] Push down filters with `Not` operator...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/22687
  
    Won't such predicates be simplified at `BooleanSimplification` rule?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #22687: [SPARK-25702][SQL] Push down filters with `Not` operator...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22687
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97193/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #22687: [SPARK-25702][SQL] Push down filters with `Not` operator...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22687
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #22687: [SPARK-25702][SQL] Push down filters with `Not` o...

Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang closed the pull request at:

    https://github.com/apache/spark/pull/22687


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #22687: [SPARK-25702][SQL] Push down filters with `Not` operator...

Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on the issue:

    https://github.com/apache/spark/pull/22687
  
    @viirya @HyukjinKwon I did the code changes and then I found the condition is not reachable, as I have stated in PR description.
    
    Just feel that it won't hurt to have such handling in data source module, the changes in code is short.
    
    I am OK to close this one.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org