Skip to content
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

[SPARK-7225][SQL] CombineLimits optimizer does not work #5770

Closed
wants to merge 16 commits into from
Closed

[SPARK-7225][SQL] CombineLimits optimizer does not work #5770

wants to merge 16 commits into from

Conversation

pzzs
Copy link
Contributor

@pzzs pzzs commented Apr 29, 2015

SQL

select key from (select key from src limit 100) t2 limit 10

Optimized Logical Plan before modifying

== Optimized Logical Plan ==
Limit 10
Limit 100
Project key#3
MetastoreRelation default, src, None

Optimized Logical Plan after modifying

== Optimized Logical Plan ==
Limit 10
 Project [key#1]
  MetastoreRelation default, src, None

pzzs added 12 commits March 2, 2015 10:11
@pzzs
Merge pull request #1 from apache/master
c3f046f 
merge lastest spark
@pzzs
Merge pull request #2 from apache/master
cb1852d 
merge lastest spark
@pzzs
Merge pull request #3 from apache/master
c87e8b6 
merge lastest spark
@pzzs
Merge pull request #4 from apache/master
161cae3 
merge lastest spark
@pzzs
Merge pull request #5 from apache/master
98b134f 
merge lastest spark
@pzzs
Merge pull request #6 from apache/master
d00303b 
merge lastest spark
@pzzs
Merge pull request #7 from apache/master
802261c 
merge lastest spark
@pzzs
Merge pull request #8 from apache/master
34b1a9a 
merge lastest spark
@pzzs
Merge pull request #9 from apache/master
f61210c 
merge lastest spark
@pzzs
Merge pull request #10 from apache/master
f12fa50 
merge lastest spark
@pzzs
Merge pull request #12 from apache/master
f03fe7f 
merge lastest spark
@pzzs
Update Optimizer.scala
e2a491d
@rxin
Copy link
Contributor

rxin commented Apr 29, 2015

Thanks - what is the change exactly that made it work? Can we have a unit test for this?

@pzzs
Copy link
Contributor Author

pzzs commented Apr 29, 2015

@rxin i have added a test fot this.

marmbrus
marmbrus reviewed Apr 30, 2015
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
PushPredicateThroughProject,
PushPredicateThroughJoin,
PushPredicateThroughGenerate,
ColumnPruning) ::
Batch("Combine Limits", FixedPoint(100),
CombineLimits) ::
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead, can we just move CombineLimits down into the Filter Pushdown batch?

Copy link
Contributor

Choose a reason for hiding this comment

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

(and rename "Filter Pushdown" into "Operator Reordering")

@pzzs
Copy link
Contributor Author

pzzs commented Apr 30, 2015

git it @marmbrus @rxin

@rxin
Copy link
Contributor

rxin commented Apr 30, 2015

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Apr 30, 2015

Test build #31377 has finished for PR 5770 at commit c68eaa7.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@rxin
Copy link
Contributor

rxin commented Apr 30, 2015

Thanks. Merging in master.

@asfgit asfgit closed this in 4459514 Apr 30, 2015
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request May 28, 2015
@pzzs @jeanlyn
[SPARK-7225][SQL] CombineLimits optimizer does not work
f787b18 
SQL
```
select key from (select key from src limit 100) t2 limit 10
```
Optimized Logical Plan before modifying
```
== Optimized Logical Plan ==
Limit 10
Limit 100
Project key#3
MetastoreRelation default, src, None
```
Optimized Logical Plan after modifying
```
== Optimized Logical Plan ==
Limit 10
 Project [key#1]
  MetastoreRelation default, src, None
```

Author: Zhongshuai Pei <799203320@qq.com>
Author: DoingDone9 <799203320@qq.com>

Closes apache#5770 from DoingDone9/limitOptimizer and squashes the following commits:

c68eaa7 [Zhongshuai Pei] Update CombiningLimitsSuite.scala
97e18cf [Zhongshuai Pei] Update Optimizer.scala
19ab875 [Zhongshuai Pei] Update CombiningLimitsSuite.scala
7db4566 [Zhongshuai Pei] Update CombiningLimitsSuite.scala
e2a491d [Zhongshuai Pei] Update Optimizer.scala
f03fe7f [Zhongshuai Pei] Merge pull request apache#12 from apache/master
f12fa50 [Zhongshuai Pei] Merge pull request apache#10 from apache/master
f61210c [Zhongshuai Pei] Merge pull request apache#9 from apache/master
34b1a9a [Zhongshuai Pei] Merge pull request apache#8 from apache/master
802261c [DoingDone9] Merge pull request apache#7 from apache/master
d00303b [DoingDone9] Merge pull request apache#6 from apache/master
98b134f [DoingDone9] Merge pull request apache#5 from apache/master
161cae3 [DoingDone9] Merge pull request apache#4 from apache/master
c87e8b6 [DoingDone9] Merge pull request apache#3 from apache/master
cb1852d [DoingDone9] Merge pull request apache#2 from apache/master
c3f046f [DoingDone9] Merge pull request apache#1 from apache/master
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
@pzzs @jeanlyn
[SPARK-7225][SQL] CombineLimits optimizer does not work
1cec5ed 
SQL
```
select key from (select key from src limit 100) t2 limit 10
```
Optimized Logical Plan before modifying
```
== Optimized Logical Plan ==
Limit 10
Limit 100
Project key#3
MetastoreRelation default, src, None
```
Optimized Logical Plan after modifying
```
== Optimized Logical Plan ==
Limit 10
 Project [key#1]
  MetastoreRelation default, src, None
```

Author: Zhongshuai Pei <799203320@qq.com>
Author: DoingDone9 <799203320@qq.com>

Closes apache#5770 from DoingDone9/limitOptimizer and squashes the following commits:

c68eaa7 [Zhongshuai Pei] Update CombiningLimitsSuite.scala
97e18cf [Zhongshuai Pei] Update Optimizer.scala
19ab875 [Zhongshuai Pei] Update CombiningLimitsSuite.scala
7db4566 [Zhongshuai Pei] Update CombiningLimitsSuite.scala
e2a491d [Zhongshuai Pei] Update Optimizer.scala
f03fe7f [Zhongshuai Pei] Merge pull request apache#12 from apache/master
f12fa50 [Zhongshuai Pei] Merge pull request apache#10 from apache/master
f61210c [Zhongshuai Pei] Merge pull request apache#9 from apache/master
34b1a9a [Zhongshuai Pei] Merge pull request apache#8 from apache/master
802261c [DoingDone9] Merge pull request apache#7 from apache/master
d00303b [DoingDone9] Merge pull request apache#6 from apache/master
98b134f [DoingDone9] Merge pull request apache#5 from apache/master
161cae3 [DoingDone9] Merge pull request apache#4 from apache/master
c87e8b6 [DoingDone9] Merge pull request apache#3 from apache/master
cb1852d [DoingDone9] Merge pull request apache#2 from apache/master
c3f046f [DoingDone9] Merge pull request apache#1 from apache/master
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
@pzzs @nemccarthy
[SPARK-7225][SQL] CombineLimits optimizer does not work
caed6bc 
SQL
```
select key from (select key from src limit 100) t2 limit 10
```
Optimized Logical Plan before modifying
```
== Optimized Logical Plan ==
Limit 10
Limit 100
Project key#3
MetastoreRelation default, src, None
```
Optimized Logical Plan after modifying
```
== Optimized Logical Plan ==
Limit 10
 Project [key#1]
  MetastoreRelation default, src, None
```

Author: Zhongshuai Pei <799203320@qq.com>
Author: DoingDone9 <799203320@qq.com>

Closes apache#5770 from DoingDone9/limitOptimizer and squashes the following commits:

c68eaa7 [Zhongshuai Pei] Update CombiningLimitsSuite.scala
97e18cf [Zhongshuai Pei] Update Optimizer.scala
19ab875 [Zhongshuai Pei] Update CombiningLimitsSuite.scala
7db4566 [Zhongshuai Pei] Update CombiningLimitsSuite.scala
e2a491d [Zhongshuai Pei] Update Optimizer.scala
f03fe7f [Zhongshuai Pei] Merge pull request apache#12 from apache/master
f12fa50 [Zhongshuai Pei] Merge pull request apache#10 from apache/master
f61210c [Zhongshuai Pei] Merge pull request apache#9 from apache/master
34b1a9a [Zhongshuai Pei] Merge pull request apache#8 from apache/master
802261c [DoingDone9] Merge pull request apache#7 from apache/master
d00303b [DoingDone9] Merge pull request apache#6 from apache/master
98b134f [DoingDone9] Merge pull request apache#5 from apache/master
161cae3 [DoingDone9] Merge pull request apache#4 from apache/master
c87e8b6 [DoingDone9] Merge pull request apache#3 from apache/master
cb1852d [DoingDone9] Merge pull request apache#2 from apache/master
c3f046f [DoingDone9] Merge pull request apache#1 from apache/master
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.
4 participants
@pzzs @rxin @SparkQA @marmbrus