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.

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

[SPARK-26576][SQL][TEST] Broadcast hint not applied to partitioned table #23530

Closed
wants to merge 2 commits into from

Conversation

jzhuge
Copy link
Member

@jzhuge jzhuge commented Jan 12, 2019

What changes were proposed in this pull request?

Make sure broadcast hint is applied to partitioned tables.

How was this patch tested?

  • A new unit test in PruneFileSourcePartitionsSuite
  • Unit test suites touched by SPARK-14581: JoinOptimizationSuite, FilterPushdownSuite, ColumnPruningSuite, and PruneFiltersSuite

Closes #23507 from jzhuge/SPARK-26576.

## What changes were proposed in this pull request?

Make sure broadcast hint is applied to partitioned tables.

## How was this patch tested?

- A new unit test in PruneFileSourcePartitionsSuite
- Unit test suites touched by SPARK-14581: JoinOptimizationSuite, FilterPushdownSuite, ColumnPruningSuite, and PruneFiltersSuite

Closes apache#23507 from jzhuge/SPARK-26576.

Authored-by: John Zhuge <jzhuge@apache.org>
Signed-off-by: gatorsmile <gatorsmile@gmail.com>
(cherry picked from commit b9eb0e8)
@dongjoon-hyun
Copy link
Member

ok to test

@@ -68,9 +68,6 @@ object PhysicalOperation extends PredicateHelper {
val substitutedCondition = substitute(aliases)(condition)
(fields, filters ++ splitConjunctivePredicates(substitutedCondition), other, aliases)

case h: ResolvedHint =>
collectProjectsAndFilters(h.child)

Copy link
Member

@dongjoon-hyun dongjoon-hyun Jan 13, 2019

Choose a reason for hiding this comment

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

Hi, @jzhuge .
Do we need to remove this? According to the previous PR, master has no issue. So, I expected a test only PR.

cc @cloud-fan and @gatorsmile

Copy link
Member Author

@jzhuge jzhuge Jan 13, 2019

Choose a reason for hiding this comment

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

Yeah, it is safer to only port the unit test. However, I believe this is dead code, otherwise we might have to revisit the fix for 2.4 and 2.3.

Copy link
Member

Choose a reason for hiding this comment

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

In that case, let's not change this in this PR [SPARK-26576][SQL] Broadcast hint not applied to partitioned table .

For the dead code cleaning up, you can do that later.

@SparkQA
Copy link

SparkQA commented Jan 13, 2019

Test build #101135 has finished for PR 23530 at commit e81a501.

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

@SparkQA
Copy link

SparkQA commented Jan 13, 2019

Test build #101139 has finished for PR 23530 at commit 434929a.

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

Copy link
Member

@gatorsmile gatorsmile left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks! Merged to master.

@gatorsmile gatorsmile changed the title [SPARK-26576][SQL] Broadcast hint not applied to partitioned table [SPARK-26576][SQL][TEST] Broadcast hint not applied to partitioned table Jan 13, 2019
@asfgit asfgit closed this in 3f80071 Jan 13, 2019
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
## What changes were proposed in this pull request?

Make sure broadcast hint is applied to partitioned tables.

## How was this patch tested?

- A new unit test in PruneFileSourcePartitionsSuite
- Unit test suites touched by SPARK-14581: JoinOptimizationSuite, FilterPushdownSuite, ColumnPruningSuite, and PruneFiltersSuite

Closes apache#23507 from jzhuge/SPARK-26576.

Closes apache#23530 from jzhuge/SPARK-26576-master.

Authored-by: John Zhuge <jzhuge@apache.org>
Signed-off-by: gatorsmile <gatorsmile@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants