Skip to content

Conversation

@gatorsmile
Copy link
Member

@gatorsmile gatorsmile commented Sep 10, 2019

What changes were proposed in this pull request?

Move the rule RemoveAllHints after the batch Resolution.

Why are the changes needed?

User-defined hints can be resolved by the rules injected via extendedResolutionRules or postHocResolutionRules.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added a test case

@SparkQA
Copy link

SparkQA commented Sep 10, 2019

Test build #110428 has finished for PR 25746 at commit 5ea0e89.

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

@AngersZhuuuu
Copy link
Contributor

AngersZhuuuu commented Sep 11, 2019

This way will be more flexible. Thanks .

@cloud-fan
Copy link
Contributor

LGTM

@dongjoon-hyun
Copy link
Member

Hi, @gatorsmile . Shall we remove [WIP]?

@dongjoon-hyun
Copy link
Member

Retest this please.

@SparkQA
Copy link

SparkQA commented Sep 18, 2019

Test build #110841 has finished for PR 25746 at commit 5ea0e89.

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

@SparkQA
Copy link

SparkQA commented Sep 18, 2019

Test build #110842 has finished for PR 25746 at commit 5ea0e89.

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

@gatorsmile
Copy link
Member Author

We need to add a test case to ensure the other user-added hints can be added and recognized.

@dongjoon-hyun
Copy link
Member

Got it. Thank you for making this change, @gatorsmile !

@gatorsmile gatorsmile changed the title [WIP][SPARK-28292][SQL] Enable Injection of User-defined Hint [SPARK-28292][SQL] Enable Injection of User-defined Hint Sep 23, 2019
@SparkQA
Copy link

SparkQA commented Sep 23, 2019

Test build #111203 has finished for PR 25746 at commit 15d3958.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmile
Copy link
Member Author

retest this please

@kiszk
Copy link
Member

kiszk commented Sep 23, 2019

nit: How was this patch tested? should be also updated now.

@SparkQA
Copy link

SparkQA commented Sep 23, 2019

Test build #111226 has finished for PR 25746 at commit 15d3958.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants