Skip to content

[SPARK-10623][SQL]: fix the predicate pushdown construction #8783

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.

Sign up for GitHub

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

Jump to bottom
Closed
zhzhan wants to merge 2 commits into from
Closed

[SPARK-10623][SQL]: fix the predicate pushdown construction #8783

zhzhan wants to merge 2 commits into from

Conversation

@zhzhan
Copy link
Contributor

@zhzhan zhzhan commented Sep 16, 2015

The predicate pushdown is not working because the construction is wrong. Fix it with startAnd/end

@SparkQA
Copy link

SparkQA commented Sep 16, 2015

Test build #42548 has finished for PR 8783 at commit 6fcc9b1.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class TaskCommitDenied(
    • final val probabilityCol: Param[String] = new Param[String](this, "probabilityCol", "Column name for predicted class conditional probabilities. Note: Not all models output well-calibrated probability estimates! These probabilities should be treated as confidences, not precise probabilities")
    • abstract class LocalNode(conf: SQLConf) extends QueryPlan[LocalNode] with Logging

@SparkQA
Copy link

SparkQA commented Sep 17, 2015

Test build #42554 has finished for PR 8783 at commit 6cec555.

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

@zhzhan
Copy link
Contributor Author

zhzhan commented Sep 17, 2015

@liancheng @marmbrus Can you help to review it?

@liancheng liancheng mentioned this pull request Sep 17, 2015
Closed
@liancheng
Copy link
Contributor

liancheng commented Sep 17, 2015

@zhzhan I just opened #8799, which is based on your PR. The reasons are:

  1. I'd like to let buildSearchArgument handle all the startAnd()/end() wrapping stuff.
  2. The in filter wasn't correctly implemented either.
  3. Add more comprehensive tests.
  4. buildSearchArgument can be further simplified for better readability.

liancheng
liancheng reviewed Sep 17, 2015
View reviewed changes
sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/OrcTest.scala
Copy link
Contributor

@liancheng liancheng Sep 17, 2015

Choose a reason for hiding this comment

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

You can use SQLTestUtils.withSQLConf instead.

@zhzhan
Copy link
Contributor Author

zhzhan commented Sep 17, 2015

@liancheng Thanks for review. Since #8799 is opened, which also fix another issue. I will close this one.

@zhzhan zhzhan closed this Sep 17, 2015
@liancheng
Copy link
Contributor

liancheng commented Sep 18, 2015

@zhzhan Thanks for investigating and working on this issue. Would you mind to help reviewing #8799?

asfgit pushed a commit that referenced this pull request Sep 19, 2015
@liancheng @yhuai
[SPARK-10623] [SQL] Fixes ORC predicate push-down
22be2ae 
When pushing down a leaf predicate, ORC `SearchArgument` builder requires an extra "parent" predicate (any one among `AND`/`OR`/`NOT`) to wrap the leaf predicate. E.g., to push down `a < 1`, we must build `AND(a < 1)` instead. Fortunately, when actually constructing the `SearchArgument`, the builder will eliminate all those unnecessary wrappers.

This PR is based on #8783 authored by zhzhan. I also took the chance to simply `OrcFilters` a little bit to improve readability.

Author: Cheng Lian <[email protected]>

Closes #8799 from liancheng/spark-10623/fix-orc-ppd.
asfgit pushed a commit that referenced this pull request Sep 19, 2015
@liancheng @yhuai
[SPARK-10623] [SQL] Fixes ORC predicate push-down
b3f1e65 
When pushing down a leaf predicate, ORC `SearchArgument` builder requires an extra "parent" predicate (any one among `AND`/`OR`/`NOT`) to wrap the leaf predicate. E.g., to push down `a < 1`, we must build `AND(a < 1)` instead. Fortunately, when actually constructing the `SearchArgument`, the builder will eliminate all those unnecessary wrappers.

This PR is based on #8783 authored by zhzhan. I also took the chance to simply `OrcFilters` a little bit to improve readability.

Author: Cheng Lian <[email protected]>

Closes #8799 from liancheng/spark-10623/fix-orc-ppd.

(cherry picked from commit 22be2ae)
Signed-off-by: Yin Huai <[email protected]>

Conflicts:
	sql/hive/src/main/scala/org/apache/spark/sql/hive/orc/OrcFilters.scala
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zhzhan @SparkQA @liancheng