[GitHub] spark issue #19451: [SPARK-22181][SQL]Adds ReplaceExceptWithFilter rule

2017-10-28 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 Thanks! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-27 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r147518995 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,104

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-27 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r147518757 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,104

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-27 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r147502424 --- Diff: sql/core/src/test/resources/sql-tests/inputs/except.sql --- @@ -0,0 +1,57 @@ +-- Tests different scenarios of except operation +create

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-27 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r147502171 --- Diff: sql/core/src/test/resources/sql-tests/inputs/except.sql --- @@ -0,0 +1,57 @@ +-- Tests different scenarios of except operation +create

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-27 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r147473160 --- Diff: sql/core/src/test/resources/sql-tests/inputs/except.sql --- @@ -0,0 +1,57 @@ +-- Tests different scenarios of except operation +create

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-27 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @gatorsmile I think i addressed all the comments, please let me know if i missed anything, thanks. --- - To unsubscribe, e

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-26 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r147269079 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,111

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-26 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r147190951 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,111

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-25 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146977529 --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PropagateEmptyRelationSuite.scala --- @@ -30,6 +30,7 @@ class

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-25 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146974572 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,111

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-25 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146973079 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,111

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-25 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146911882 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,111

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-25 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146910095 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,111

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-25 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146907573 --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PropagateEmptyRelationSuite.scala --- @@ -30,6 +30,7 @@ class

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-25 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146906157 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala --- @@ -1200,6 +1211,8 @@ class SQLConf extends Serializable

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-24 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @dilipbiswal @gatorsmile I included all your comments, please let me know if i missed anything, thanks. --- - To unsubscribe

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-24 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146689987 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,114

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-24 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146617224 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,114

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-24 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146606645 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,114

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-24 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146606031 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,114

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-24 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146603041 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,114

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-23 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146384601 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,114

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-23 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146377893 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,114

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-23 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146377175 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,114

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-23 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r146376886 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,114

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-18 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @gatorsmile please let me know if you have comments regarding the new version of the rule, thanks. --- - To unsubscribe, e

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-17 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @gatorsmile done! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-16 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @gatorsmile you are right! actually i forgot to push the updates in the test suite. I pushed it now and the test cases are passing

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-16 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 And could anybody please give me the needed rights to assign this task to myself in the jira? thanks. --- - To unsubscribe, e

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-16 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r144913210 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceExceptWithFilter.scala --- @@ -0,0 +1,73

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-16 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 In the previous version of the rule, the `InferFiltersFromConstraints` rule added some additional filters (like 'col.isNotNull) after the filter condition is flipped, so it filtered out the null

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-16 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @gengliangwang thank you very much! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-15 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @gatorsmile one quick question, could you please say which rule is transforming the filter conditions like `'b === 2` to `'b.isNotNull && 'b === 2`

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-13 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @rxin I think it would be better to keep all the rules of the "Replace Operators" batch in a single file. So if you prefer to keep the rule in a new file, we can move all the replac

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-13 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @gatorsmile > Could you please add an end-to-end testsuite except.sql of SQLQueryTestSuite.scala? Please verify `except.sql ` and `except.sql.out` files are enough for the

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-12 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @gengliangwang thanks! I'll squash my commits into 2 if everything is fine.. --- - To unsubscribe, e-mail: reviews

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-11 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @gengliangwang Ready for a next review :) > put case ... in a new line Are your sure? I thought according to the coding style, while calling on a partial funct

[GitHub] spark issue #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-10 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19451 @gengliangwang thanks for your review. I pushed a new commit, please let me know if it addresses your comments. And regarding > Also please use semanticEquals instead of sameRes

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-06 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19451#discussion_r143289752 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala --- @@ -1242,6 +1243,53 @@ object

[GitHub] spark pull request #19451: SPARK-22181 Adds ReplaceExceptWithNotFilter rule

2017-10-06 Thread sathiyapk
GitHub user sathiyapk opened a pull request: https://github.com/apache/spark/pull/19451 SPARK-22181 Adds ReplaceExceptWithNotFilter rule ## What changes were proposed in this pull request? Adds a new optimisation rule 'ReplaceExceptWithNotFilter' that replaces Except

[GitHub] spark issue #19295: [SPARK-22080][SQL] Adds support for allowing user to add...

2017-09-24 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19295 I pushed a new commit that addresses @wzhfy review comments.. --- - To unsubscribe, e-mail: reviews-unsubscr

[GitHub] spark pull request #19295: [SPARK-22080][SQL] Adds support for allowing user...

2017-09-24 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19295#discussion_r140652720 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/ExperimentalMethods.scala --- @@ -44,11 +44,14 @@ class ExperimentalMethods private[sql

[GitHub] spark issue #19295: [SPARK-22080][SQL] Adds support for allowing user to add...

2017-09-24 Thread sathiyapk
Github user sathiyapk commented on the issue: https://github.com/apache/spark/pull/19295 @gatorsmile thanks for your comments. Here are my thoughts, thanks for correcting me if i'm wrong. (sorry for the big comment though :)) 1. This PR don't change any existing API, it adds a new

[GitHub] spark pull request #19295: [SPARK-22080][SQL] Adds support for allowing user...

2017-09-22 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19295#discussion_r140464377 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkOptimizer.scala --- @@ -28,12 +28,18 @@ class SparkOptimizer

[GitHub] spark pull request #19295: [SPARK-22080][SQL] Adds support for allowing user...

2017-09-22 Thread sathiyapk
Github user sathiyapk commented on a diff in the pull request: https://github.com/apache/spark/pull/19295#discussion_r140450459 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkOptimizer.scala --- @@ -28,12 +28,18 @@ class SparkOptimizer

[GitHub] spark pull request #19295: SPARK-22080 Adds support for allowing user to add...

2017-09-20 Thread sathiyapk
GitHub user sathiyapk opened a pull request: https://github.com/apache/spark/pull/19295 SPARK-22080 Adds support for allowing user to add pre-optimization rules ## What changes were proposed in this pull request? Currently the user provided custom rules are applied only