Skip to content

Conversation

@maryannxue
Copy link
Contributor

What changes were proposed in this pull request?

This is an extension to the original PR, in which rule exclusion did not work for classes derived from Optimizer, e.g., SparkOptimizer.
To solve this issue, Optimizer and its derived classes will define/override defaultBatches and nonExcludableRules in order to define its default rule set as well as rules that cannot be excluded by the SQL config. In the meantime, Optimizer's batches method is dedicated to the rule exclusion logic and is defined "final".

How was this patch tested?

Added UT.

@SparkQA
Copy link

SparkQA commented Jul 26, 2018

Test build #93571 has finished for PR 21876 at commit 3730053.

  • 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

@gatorsmile
Copy link
Member

Thanks! Merged to master

@asfgit asfgit closed this in 5ed7660 Jul 26, 2018
markhamstra pushed a commit to markhamstra/spark that referenced this pull request Jul 27, 2018
…Exclusion

## What changes were proposed in this pull request?

This is an extension to the original PR, in which rule exclusion did not work for classes derived from Optimizer, e.g., SparkOptimizer.
To solve this issue, Optimizer and its derived classes will define/override `defaultBatches` and `nonExcludableRules` in order to define its default rule set as well as rules that cannot be excluded by the SQL config. In the meantime, Optimizer's `batches` method is dedicated to the rule exclusion logic and is defined "final".

## How was this patch tested?

Added UT.

Author: maryannxue <maryannxue@apache.org>

Closes apache#21876 from maryannxue/rule-exclusion.
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.

3 participants