You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/08/26 00:57:38 UTC

[GitHub] [spark] maropu opened a new pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

maropu opened a new pull request #29544:
URL: https://github.com/apache/spark/pull/29544


   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
     7. If you want to add a new configuration, please read the guideline first for naming configurations in
        'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   Since we only log plan changes for analyzer/optimizer now, this PR intends to add code to log plan changes in the preparation phase in `QueryExecution` for execution.
   ```
   scala> spark.sql("SET spark.sql.optimizer.planChangeLog.level=WARN")
   scala> spark.range(10).groupBy("id").count().queryExecution.executedPlan
   ...
   20/08/26 09:32:36 WARN PlanChangeLogger: 
   === Applying Rule org.apache.spark.sql.execution.CollapseCodegenStages ===
   !HashAggregate(keys=[id#19L], functions=[count(1)], output=[id#19L, count#23L])              *(1) HashAggregate(keys=[id#19L], functions=[count(1)], output=[id#19L, count#23L])
   !+- HashAggregate(keys=[id#19L], functions=[partial_count(1)], output=[id#19L, count#27L])   +- *(1) HashAggregate(keys=[id#19L], functions=[partial_count(1)], output=[id#19L, count#27L])
   !   +- Range (0, 10, step=1, splits=4)                                                          +- *(1) Range (0, 10, step=1, splits=4)
            
   20/08/26 09:32:36 WARN PlanChangeLogger: 
   === Result of Batch Preparations ===
   !HashAggregate(keys=[id#19L], functions=[count(1)], output=[id#19L, count#23L])              *(1) HashAggregate(keys=[id#19L], functions=[count(1)], output=[id#19L, count#23L])
   !+- HashAggregate(keys=[id#19L], functions=[partial_count(1)], output=[id#19L, count#27L])   +- *(1) HashAggregate(keys=[id#19L], functions=[partial_count(1)], output=[id#19L, count#27L])
   !   +- Range (0, 10, step=1, splits=4)                                                          +- *(1) Range (0, 10, step=1, splits=4)
   ```
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   Easy debugging for executed plans
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   No.
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   -->
   Added unit tests.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682264651


   **[Test build #127972 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127972/testReport)** for PR 29544 at commit [`24aec4c`](https://github.com/apache/spark/commit/24aec4c59b94830eb8e0a2f7131f34986b04864b).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682339666






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682351032


   **[Test build #127984 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127984/testReport)** for PR 29544 at commit [`40c99a2`](https://github.com/apache/spark/commit/40c99a21ae19f4c1e50029e4b10c768339336cc6).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r489280396



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala
##########
@@ -359,7 +359,14 @@ object QueryExecution {
   private[execution] def prepareForExecution(
       preparations: Seq[Rule[SparkPlan]],
       plan: SparkPlan): SparkPlan = {
-    preparations.foldLeft(plan) { case (sp, rule) => rule.apply(sp) }
+    val planChangeLogger = new PlanChangeLogger[SparkPlan]()

Review comment:
       We also run physical rules in `AdaptiveSparkPlanExec`, shall we log plan change there as well?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682351032


   **[Test build #127984 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127984/testReport)** for PR 29544 at commit [`40c99a2`](https://github.com/apache/spark/commit/40c99a21ae19f4c1e50029e4b10c768339336cc6).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r478776434



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
##########
@@ -153,11 +220,12 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging {
             if (effective) {
               queryExecutionMetrics.incNumEffectiveExecution(rule.ruleName)
               queryExecutionMetrics.incTimeEffectiveExecutionBy(rule.ruleName, runTime)
-              planChangeLogger.logRule(rule.ruleName, plan, result)
             }
             queryExecutionMetrics.incExecutionTimeBy(rule.ruleName, runTime)
             queryExecutionMetrics.incNumExecution(rule.ruleName)
 
+            planChangeLogger.logRule(rule.ruleName, plan, result)
+

Review comment:
       Move out of `if (effective) { ... }` by intention? Won't it be flooded with too many log?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680396068






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680396068






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682351678






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682339666






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682264930






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680637675


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/127902/
   Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r478231857



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala
##########
@@ -359,7 +359,17 @@ object QueryExecution {
   private[execution] def prepareForExecution(
       preparations: Seq[Rule[SparkPlan]],
       plan: SparkPlan): SparkPlan = {
-    preparations.foldLeft(plan) { case (sp, rule) => rule.apply(sp) }
+    val planChangeLogger = new PlanChangeLogger[SparkPlan]()
+    val preparedPlan = preparations.foldLeft(plan) { case (sp, rule) =>
+      val result = rule.apply(sp)
+      val effective = !result.fastEquals(sp)

Review comment:
       shall we check this inside the `planChangeLogger.logRule`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682341541






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] gatorsmile commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
gatorsmile commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r503049350



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -229,17 +229,17 @@ object SQLConf {
         "'trace', 'debug', 'info', 'warn' and 'error'.")
     .createWithDefault("trace")
 
-  val OPTIMIZER_PLAN_CHANGE_LOG_RULES = buildConf("spark.sql.optimizer.planChangeLog.rules")
+  val PLAN_CHANGE_LOG_RULES = buildConf("spark.sql.planChangeLog.rules")
     .internal()
-    .doc("Configures a list of rules to be logged in the optimizer, in which the rules are " +
+    .doc("Configures a list of rules for logging plan changes, in which the rules are " +
       "specified by their rule names and separated by comma.")
     .version("3.0.0")
     .stringConf
     .createOptional
 
-  val OPTIMIZER_PLAN_CHANGE_LOG_BATCHES = buildConf("spark.sql.optimizer.planChangeLog.batches")
+  val PLAN_CHANGE_LOG_BATCHES = buildConf("spark.sql.planChangeLog.batches")
     .internal()
-    .doc("Configures a list of batches to be logged in the optimizer, in which the batches " +
+    .doc("Configures a list of batches for logging plan changes, in which the batches " +
       "are specified by their batch names and separated by comma.")
     .version("3.0.0")

Review comment:
       The version of these three configurations needs to be updated to 3.1.0? 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682339121


   **[Test build #127974 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127974/testReport)** for PR 29544 at commit [`eb3cbb0`](https://github.com/apache/spark/commit/eb3cbb0f2c9d1ceb569b6644eb0f910f88e9f3ee).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682672042


   **[Test build #127996 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127996/testReport)** for PR 29544 at commit [`40c99a2`](https://github.com/apache/spark/commit/40c99a21ae19f4c1e50029e4b10c768339336cc6).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680701274


   **[Test build #127915 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127915/testReport)** for PR 29544 at commit [`54029d5`](https://github.com/apache/spark/commit/54029d536c79ab37369d7a3998440f96ddeea90e).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682266505


   **[Test build #127974 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127974/testReport)** for PR 29544 at commit [`eb3cbb0`](https://github.com/apache/spark/commit/eb3cbb0f2c9d1ceb569b6644eb0f910f88e9f3ee).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r478824595



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
##########
@@ -44,6 +44,73 @@ object RuleExecutor {
   }
 }
 
+class PlanChangeLogger[TreeType <: TreeNode[_]] extends Logging {
+
+  private val logLevel = SQLConf.get.planChangeLogLevel
+
+  private val logRules = SQLConf.get.planChangeRules.map(Utils.stringToSeq)
+
+  private val logBatches = SQLConf.get.planChangeBatches.map(Utils.stringToSeq)
+
+  def logRule(ruleName: String, oldPlan: TreeType, newPlan: TreeType): Unit = {
+    if (!newPlan.fastEquals(oldPlan)) {
+      if (logRules.isEmpty || logRules.get.contains(ruleName)) {
+        def message(): String = {
+          s"""
+             |=== Applying Rule $ruleName ===
+             |${sideBySide(oldPlan.treeString, newPlan.treeString).mkString("\n")}
+           """.stripMargin
+        }

Review comment:
       shall we follow https://github.com/apache/spark/pull/29544/files#diff-f70523b948b7af21abddfa3ab7e1d7d6R79 and log something like `Rule xxx has no effect`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-683112325






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682266810






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682264651


   **[Test build #127972 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127972/testReport)** for PR 29544 at commit [`24aec4c`](https://github.com/apache/spark/commit/24aec4c59b94830eb8e0a2f7131f34986b04864b).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680701274


   **[Test build #127915 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127915/testReport)** for PR 29544 at commit [`54029d5`](https://github.com/apache/spark/commit/54029d536c79ab37369d7a3998440f96ddeea90e).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] maropu commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r489439735



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala
##########
@@ -359,7 +359,14 @@ object QueryExecution {
   private[execution] def prepareForExecution(
       preparations: Seq[Rule[SparkPlan]],
       plan: SparkPlan): SparkPlan = {
-    preparations.foldLeft(plan) { case (sp, rule) => rule.apply(sp) }
+    val planChangeLogger = new PlanChangeLogger[SparkPlan]()

Review comment:
       https://github.com/apache/spark/pull/29774




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680637654


   Merged build finished. Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] maropu commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r478770188



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
##########
@@ -44,6 +44,71 @@ object RuleExecutor {
   }
 }
 
+class PlanChangeLogger[TreeType <: TreeNode[_]] extends Logging {
+
+  private val logLevel = SQLConf.get.optimizerPlanChangeLogLevel
+
+  private val logRules = SQLConf.get.optimizerPlanChangeRules.map(Utils.stringToSeq)
+
+  private val logBatches = SQLConf.get.optimizerPlanChangeBatches.map(Utils.stringToSeq)

Review comment:
       Ah, okay.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682351678






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682672042


   **[Test build #127996 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127996/testReport)** for PR 29544 at commit [`40c99a2`](https://github.com/apache/spark/commit/40c99a21ae19f4c1e50029e4b10c768339336cc6).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan closed pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #29544:
URL: https://github.com/apache/spark/pull/29544


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680835756






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682674290






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680637654






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682674290






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682660835


   retest this please


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682340761


   **[Test build #127972 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127972/testReport)** for PR 29544 at commit [`24aec4c`](https://github.com/apache/spark/commit/24aec4c59b94830eb8e0a2f7131f34986b04864b).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] maropu commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-681173222


   cc: @cloud-fan @viirya 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-683110928


   **[Test build #127996 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127996/testReport)** for PR 29544 at commit [`40c99a2`](https://github.com/apache/spark/commit/40c99a21ae19f4c1e50029e4b10c768339336cc6).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680701716






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682365673


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/127984/
   Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682365583


   **[Test build #127984 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127984/testReport)** for PR 29544 at commit [`40c99a2`](https://github.com/apache/spark/commit/40c99a21ae19f4c1e50029e4b10c768339336cc6).
    * This patch **fails due to an unknown error code, -9**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682843660


   GH action passed, merging to master, thanks!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682365664






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] viirya commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r478776982



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
##########
@@ -153,11 +220,12 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging {
             if (effective) {
               queryExecutionMetrics.incNumEffectiveExecution(rule.ruleName)
               queryExecutionMetrics.incTimeEffectiveExecutionBy(rule.ruleName, runTime)
-              planChangeLogger.logRule(rule.ruleName, plan, result)
             }
             queryExecutionMetrics.incExecutionTimeBy(rule.ruleName, runTime)
             queryExecutionMetrics.incNumExecution(rule.ruleName)
 
+            planChangeLogger.logRule(rule.ruleName, plan, result)
+

Review comment:
       Oh, nvm, I saw the previous comment.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r478825044



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -216,7 +216,7 @@ object SQLConf {
         "for using switch statements in InSet must be non-negative and less than or equal to 600")
       .createWithDefault(400)
 
-  val OPTIMIZER_PLAN_CHANGE_LOG_LEVEL = buildConf("spark.sql.optimizer.planChangeLog.level")
+  val PLAN_CHANGE_LOG_LEVEL = buildConf("spark.sql.planChangeLog.level")

Review comment:
       can we put the old config name in `removedSQLConfigs`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] maropu commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r503051134



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -229,17 +229,17 @@ object SQLConf {
         "'trace', 'debug', 'info', 'warn' and 'error'.")
     .createWithDefault("trace")
 
-  val OPTIMIZER_PLAN_CHANGE_LOG_RULES = buildConf("spark.sql.optimizer.planChangeLog.rules")
+  val PLAN_CHANGE_LOG_RULES = buildConf("spark.sql.planChangeLog.rules")
     .internal()
-    .doc("Configures a list of rules to be logged in the optimizer, in which the rules are " +
+    .doc("Configures a list of rules for logging plan changes, in which the rules are " +
       "specified by their rule names and separated by comma.")
     .version("3.0.0")
     .stringConf
     .createOptional
 
-  val OPTIMIZER_PLAN_CHANGE_LOG_BATCHES = buildConf("spark.sql.optimizer.planChangeLog.batches")
+  val PLAN_CHANGE_LOG_BATCHES = buildConf("spark.sql.planChangeLog.batches")
     .internal()
-    .doc("Configures a list of batches to be logged in the optimizer, in which the batches " +
+    .doc("Configures a list of batches for logging plan changes, in which the batches " +
       "are specified by their batch names and separated by comma.")
     .version("3.0.0")

Review comment:
       Ah, I missed it. I'll update it in followup.

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -229,17 +229,17 @@ object SQLConf {
         "'trace', 'debug', 'info', 'warn' and 'error'.")
     .createWithDefault("trace")
 
-  val OPTIMIZER_PLAN_CHANGE_LOG_RULES = buildConf("spark.sql.optimizer.planChangeLog.rules")
+  val PLAN_CHANGE_LOG_RULES = buildConf("spark.sql.planChangeLog.rules")
     .internal()
-    .doc("Configures a list of rules to be logged in the optimizer, in which the rules are " +
+    .doc("Configures a list of rules for logging plan changes, in which the rules are " +
       "specified by their rule names and separated by comma.")
     .version("3.0.0")
     .stringConf
     .createOptional
 
-  val OPTIMIZER_PLAN_CHANGE_LOG_BATCHES = buildConf("spark.sql.optimizer.planChangeLog.batches")
+  val PLAN_CHANGE_LOG_BATCHES = buildConf("spark.sql.planChangeLog.batches")
     .internal()
-    .doc("Configures a list of batches to be logged in the optimizer, in which the batches " +
+    .doc("Configures a list of batches for logging plan changes, in which the batches " +
       "are specified by their batch names and separated by comma.")
     .version("3.0.0")

Review comment:
       Ah, I missed it. I'll update it in followup. Thanks!




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682266505


   **[Test build #127974 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127974/testReport)** for PR 29544 at commit [`eb3cbb0`](https://github.com/apache/spark/commit/eb3cbb0f2c9d1ceb569b6644eb0f910f88e9f3ee).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r478232514



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
##########
@@ -44,6 +44,71 @@ object RuleExecutor {
   }
 }
 
+class PlanChangeLogger[TreeType <: TreeNode[_]] extends Logging {
+
+  private val logLevel = SQLConf.get.optimizerPlanChangeLogLevel
+
+  private val logRules = SQLConf.get.optimizerPlanChangeRules.map(Utils.stringToSeq)
+
+  private val logBatches = SQLConf.get.optimizerPlanChangeBatches.map(Utils.stringToSeq)

Review comment:
       shall we rename the config names? just remove `optimizer`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-683112325






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682264930






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680394103


   **[Test build #127902 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127902/testReport)** for PR 29544 at commit [`a767c76`](https://github.com/apache/spark/commit/a767c76b2bcefa2b3acf5850151a97c223dd949a).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680835756






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682365664


   Merged build finished. Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680635321


   **[Test build #127902 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127902/testReport)** for PR 29544 at commit [`a767c76`](https://github.com/apache/spark/commit/a767c76b2bcefa2b3acf5850151a97c223dd949a).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `class PlanChangeLogger [TreeType <: TreeNode[_]] extends Logging `


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] maropu commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r489320333



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala
##########
@@ -359,7 +359,14 @@ object QueryExecution {
   private[execution] def prepareForExecution(
       preparations: Seq[Rule[SparkPlan]],
       plan: SparkPlan): SparkPlan = {
-    preparations.foldLeft(plan) { case (sp, rule) => rule.apply(sp) }
+    val planChangeLogger = new PlanChangeLogger[SparkPlan]()

Review comment:
       Yea, sure. I'll make a PR later.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680834767


   **[Test build #127915 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127915/testReport)** for PR 29544 at commit [`54029d5`](https://github.com/apache/spark/commit/54029d536c79ab37369d7a3998440f96ddeea90e).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `class PlanChangeLogger[TreeType <: TreeNode[_]] extends Logging `


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] maropu commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682274018


   Thanks for the review, @viirya !


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680701716






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682341541






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] AmplabJenkins removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-682266810






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] cloud-fan commented on a change in pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29544:
URL: https://github.com/apache/spark/pull/29544#discussion_r478824806



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
##########
@@ -153,11 +220,12 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging {
             if (effective) {
               queryExecutionMetrics.incNumEffectiveExecution(rule.ruleName)
               queryExecutionMetrics.incTimeEffectiveExecutionBy(rule.ruleName, runTime)
-              planChangeLogger.logRule(rule.ruleName, plan, result)

Review comment:
       It's OK to still put it here.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] SparkQA removed a comment on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680394103


   **[Test build #127902 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127902/testReport)** for PR 29544 at commit [`a767c76`](https://github.com/apache/spark/commit/a767c76b2bcefa2b3acf5850151a97c223dd949a).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] HyukjinKwon commented on pull request #29544: [SPARK-32704][SQL] Logging plan changes for execution

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #29544:
URL: https://github.com/apache/spark/pull/29544#issuecomment-680551008


   +1!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org