Skip to content

[SPARK-13527] [SQL] Prune Filters based on Constraints #11406

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
wants to merge 9 commits into from
Closed

[SPARK-13527] [SQL] Prune Filters based on Constraints #11406

wants to merge 9 commits into from

Conversation

@gatorsmile
Copy link
Member

@gatorsmile gatorsmile commented Feb 27, 2016

What changes were proposed in this pull request?

Remove all the deterministic conditions in a [[Filter]] that are contained in the Child's Constraints.

For example, the first query can be simplified to the second one.

    val queryWithUselessFilter = tr1
      .where("tr1.a".attr > 10 || "tr1.c".attr < 10)
      .join(tr2.where('d.attr < 100), Inner, Some("tr1.a".attr === "tr2.a".attr))
      .where(
        ("tr1.a".attr > 10 || "tr1.c".attr < 10) &&
        'd.attr < 100 &&
        "tr2.a".attr === "tr1.a".attr)
    val query = tr1
      .where("tr1.a".attr > 10 || "tr1.c".attr < 10)
      .join(tr2.where('d.attr < 100), Inner, Some("tr1.a".attr === "tr2.a".attr))

How was this patch tested?

Six test cases are added.

@gatorsmile gatorsmile mentioned this pull request Feb 27, 2016
Closed
@SparkQA
Copy link

SparkQA commented Feb 27, 2016

Test build #52095 has finished for PR 11406 at commit e50d307.

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

@SparkQA
Copy link

SparkQA commented Feb 27, 2016

Test build #52096 has finished for PR 11406 at commit f889c91.

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

rxin
rxin reviewed Feb 27, 2016
View reviewed changes
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/FilterPushdownSuite.scala Outdated
comparePlans(optimized, correctAnswer)
}

test("Filter removal #1 -- isNull + LeftOuter") {
Copy link
Contributor

@rxin rxin Feb 27, 2016

Choose a reason for hiding this comment

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

PruneFiltersSuite?

Copy link
Member Author

@gatorsmile gatorsmile Feb 27, 2016

Choose a reason for hiding this comment

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

sure. will do

@SparkQA
Copy link

SparkQA commented Feb 27, 2016

Test build #52111 has finished for PR 11406 at commit b9e68b9.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class PruneFiltersSuite extends PlanTest

@SparkQA
Copy link

SparkQA commented Feb 27, 2016

Test build #52112 has finished for PR 11406 at commit c3a504e.

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

@gatorsmile
Copy link
Member Author

gatorsmile commented Mar 2, 2016

@marmbrus @sameeragarwal Could you please take a look at this rule? Thanks!

marmbrus
marmbrus reviewed Mar 4, 2016
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala Outdated
} else if (remainingPredicates.isEmpty) {
p
} else {
val newCond = remainingPredicates.reduceOption(And).getOrElse(Literal(true))
Copy link
Contributor

@marmbrus marmbrus Mar 4, 2016

Choose a reason for hiding this comment

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

I don't think that you need the type ascription (: LogicalPlan) here or above in the match.

Copy link
Member Author

@gatorsmile gatorsmile Mar 5, 2016

Choose a reason for hiding this comment

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

will remove it. Thanks!

@SparkQA
Copy link

SparkQA commented Mar 5, 2016

Test build #52509 has finished for PR 11406 at commit b71c084.

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

@SparkQA
Copy link

SparkQA commented Mar 7, 2016

Test build #52583 has finished for PR 11406 at commit 83e0f4a.

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

viirya
viirya reviewed Mar 8, 2016
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
/**
* Remove all the deterministic conditions in a [[Filter]] that are guaranteed to be true
* given the constraints on the child's output.
*/
Copy link
Member

@viirya viirya Mar 8, 2016

Choose a reason for hiding this comment

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

Looks like SimplifyFilters is similar in purpose with this rule. Can we merge them?

Copy link
Member Author

@gatorsmile gatorsmile Mar 8, 2016

Choose a reason for hiding this comment

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

Sure, will do it. Thanks!

@SparkQA
Copy link

SparkQA commented Mar 8, 2016

Test build #52632 has finished for PR 11406 at commit 7b7b411.

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

@gatorsmile
Copy link
Member Author

gatorsmile commented Mar 9, 2016

cc @marmbrus Any comment about this?

After this is merged, I can finish the outer join elimination: #10566

Thanks!

@marmbrus
Copy link
Contributor

marmbrus commented Mar 9, 2016

Thanks, merging to master.

@asfgit asfgit closed this in c6aa356 Mar 9, 2016
roygao94 pushed a commit to roygao94/spark that referenced this pull request Mar 22, 2016
@gatorsmile @roygao94
[SPARK-13527][SQL] Prune Filters based on Constraints
22f5565 
#### What changes were proposed in this pull request?

Remove all the deterministic conditions in a [[Filter]] that are contained in the Child's Constraints.

For example, the first query can be simplified to the second one.

```scala
    val queryWithUselessFilter = tr1
      .where("tr1.a".attr > 10 || "tr1.c".attr < 10)
      .join(tr2.where('d.attr < 100), Inner, Some("tr1.a".attr === "tr2.a".attr))
      .where(
        ("tr1.a".attr > 10 || "tr1.c".attr < 10) &&
        'd.attr < 100 &&
        "tr2.a".attr === "tr1.a".attr)
```
```scala
    val query = tr1
      .where("tr1.a".attr > 10 || "tr1.c".attr < 10)
      .join(tr2.where('d.attr < 100), Inner, Some("tr1.a".attr === "tr2.a".attr))
```
#### How was this patch tested?

Six test cases are added.

Author: gatorsmile <[email protected]>

Closes apache#11406 from gatorsmile/FilterRemoval.
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.

5 participants

@gatorsmile @SparkQA @marmbrus @viirya @rxin