[SPARK-32717][SQL] Add a AQEOptimizer for AdaptiveSparkPlanExec

[Ngone51]

What changes were proposed in this pull request?

This PR proposes to add a specific AQEOptimizer for the AdaptiveSparkPlanExec instead of implementing an anonymous RuleExecutor. At the same time, this PR also adds the configuration spark.sql.adaptive.optimizer.excludedRules, which follows the same pattern of Optimizer, to make the AQEOptimizer more flexible for users and developers.

Why are the changes needed?

Currently, AdaptiveSparkPlanExec has implemented an anonymous RuleExecutor to apply the AQE optimize rules on the plan. However, the anonymous class usually could be inconvenient to maintain and extend for the long term.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

It's a pure refactor so pass existing tests should be ok.

[Ngone51]

@cloud-fan @maryannxue Please take a look, thanks!

[SparkQA]

Test build #127955 has finished for PR 29559 at commit 291db45.

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

[SparkQA]

Test build #127956 has finished for PR 29559 at commit 9f4d3b5.

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

[c21]

Just wondering why we need to introduce a config for user to disable rule in AQE? Is there a use case we are considering now? I am seeing spark.sql.optimizer.excludedRules was introduced to work around https://issues.apache.org/jira/browse/SPARK-24624 (#21764 (comment)), but not sure what's the use case here for AQE.

[Ngone51]

@c21 @maropu Thank you for your comments. spark.sql.adaptive.optimizer.excludedRule doesn't have a real use case yet, but I think it's quite intuitive to follow the normal optimizer given the same reason(more flexible for users). I also think it can be very useful for developers to write certain tests in AQE. For example, If they want to avoid applying the rule DemoteBroadcastHashJoin. Previously, they have to tweak the size of the test data and the partition num, which can be troublesome. With this configuration, they can just exclude the rule easily.

[maropu]

yea, adding it itself looks okay to me.

[c21]

@Ngone51 - thanks, the rationale for developer makes sense to me.

[Ngone51]

Thanks, I've added a unit test and updated the PR description for the rule excluding configuration.

[SparkQA]

Test build #127991 has finished for PR 29559 at commit cfd348e.

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

[HyukjinKwon]

Merged to master.

[Ngone51]

thanks all!