vladimirg-db commented on code in PR #49271:
URL: https://github.com/apache/spark/pull/49271#discussion_r1895810076


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/PlanLogger.scala:
##########
@@ -27,32 +28,64 @@ import org.apache.spark.sql.internal.SQLConf
  * [[PlanLogger]] is used by the [[Resolver]] to log intermediate resolution 
results.
  */
 class PlanLogger extends Logging {
-  private val logLevel = SQLConf.get.planChangeLogLevel
+  private val planChangeLogLevel = SQLConf.get.planChangeLogLevel
+  private val expressionTreeChangeLogLevel = 
SQLConf.get.expressionTreeChangeLogLevel
 
-  /**
-   * Logs the transition from the `unresolvedPlan` to the `resolvedPlan`.
-   */
-  def log(unresolvedPlan: LogicalPlan, resolvedPlan: LogicalPlan): Unit = {
-    logBasedOnLevel(() => createMessage(unresolvedPlan, resolvedPlan))
+  def logPlanResolutionEvent(plan: LogicalPlan, event: String): Unit = {
+    log(() => log"""
+       |=== Plan resolution: ${MDC(MESSAGE, event)} ===
+       |${MDC(QUERY_PLAN, plan.treeString)}
+     """.stripMargin, planChangeLogLevel)
   }
 
-  private def createMessage(
-      unresolvedPlan: LogicalPlan,
-      resolvedPlan: LogicalPlan): MessageWithContext =
-    log"""
-       |=== Unresolved/resolved operator subtree ===
+  def logPlanResolution(unresolvedPlan: LogicalPlan, resolvedPlan: 
LogicalPlan): Unit = {
+    log(
+      () =>
+        log"""
+       |=== Unresolved plan -> Resolved plan ===
        |${MDC(
-           QUERY_PLAN,
-           sideBySide(unresolvedPlan.treeString, 
resolvedPlan.treeString).mkString("\n")
-         )}
-     """.stripMargin
-
-  private def logBasedOnLevel(createMessage: () => MessageWithContext): Unit = 
logLevel match {
-    case "TRACE" => logTrace(createMessage().message)
-    case "DEBUG" => logDebug(createMessage().message)
-    case "INFO" => logInfo(createMessage())
-    case "WARN" => logWarning(createMessage())
-    case "ERROR" => logError(createMessage())
-    case _ => logTrace(createMessage().message)
+               QUERY_PLAN,
+               sideBySide(unresolvedPlan.treeString, 
resolvedPlan.treeString).mkString("\n")
+             )}
+     """.stripMargin,
+      planChangeLogLevel
+    )
   }
+
+  def logExpressionTreeResolutionEvent(expressionTree: Expression, event: 
String): Unit = {
+    log(
+      () => log"""
+       |=== Expression tree resolution: ${MDC(MESSAGE, event)} ===
+       |${MDC(QUERY_PLAN, expressionTree.treeString)}
+     """.stripMargin,
+      expressionTreeChangeLogLevel
+    )
+  }
+
+  def logExpressionTreeResolution(
+      unresolvedExpressionTree: Expression,

Review Comment:
   Yup, we do that. At first i just added the `unresolved -> resolved` logging 
(as you can see). However, from my experience debugging the single-pass 
Analyzer this is much more useful than just seeing upwards resolution, because:
   1. You can see the initial unresolved tree
   2. You can see the resolution order the tree traversal takes when it 
proceeds downwards to the leaves



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to