[GitHub] [spark] dongjoon-hyun commented on a change in pull request #26898: [SPARK-30200][SQL][FOLLOW-UP] Expose only explain(mode: String) in Scala side, and clean up related codes

[GitBox <gi...@apache.org>]

dongjoon-hyun commented on a change in pull request #26898: [SPARK-30200][SQL][FOLLOW-UP] Expose only explain(mode: String) in Scala side, and clean up related codes
URL: https://github.com/apache/spark/pull/26898#discussion_r358045434
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala
 ##########
 @@ -522,53 +521,20 @@ class Dataset[T] private[sql](
   def printSchema(level: Int): Unit = println(schema.treeString(level))
   // scalastyle:on println
 
-  private def toExplainString(mode: ExplainMode): String = {
-    // Because temporary views are resolved during analysis when we create a Dataset, and
-    // `ExplainCommand` analyzes input query plan and resolves temporary views again. Using
-    // `ExplainCommand` here will probably output different query plans, compared to the results
-    // of evaluation of the Dataset. So just output QueryExecution's query plans here.
-    val qe = ExplainCommandUtil.explainedQueryExecution(sparkSession, logicalPlan, queryExecution)
-
-    mode match {
-      case ExplainMode.Simple =>
-        qe.simpleString
-      case ExplainMode.Extended =>
-        qe.toString
-      case ExplainMode.Codegen =>
-        try {
-          org.apache.spark.sql.execution.debug.codegenString(queryExecution.executedPlan)
-        } catch {
-          case e: AnalysisException => e.toString
-        }
-      case ExplainMode.Cost =>
-        qe.stringWithStats
-      case ExplainMode.Formatted =>
-        qe.simpleString(formatted = true)
-    }
-  }
-
-  // This method intends to be called from PySpark DataFrame
-  private[sql] def toExplainString(mode: String): String = {
-    mode.toLowerCase(Locale.ROOT) match {
-      case "simple" => toExplainString(ExplainMode.Simple)
-      case "extended" => toExplainString(ExplainMode.Extended)
-      case "codegen" => toExplainString(ExplainMode.Codegen)
-      case "cost" => toExplainString(ExplainMode.Cost)
-      case "formatted" => toExplainString(ExplainMode.Formatted)
-      case _ => throw new IllegalArgumentException(s"Unknown explain mode: $mode. Accepted " +
-        "explain modes are 'simple', 'extended', 'codegen', 'cost', 'formatted'.")
-    }
-  }
-
   /**
    * Prints the plans (logical and physical) with a format specified by a given explain mode.
    *
    * @group basic
    * @since 3.0.0
    */
-  def explain(mode: ExplainMode): Unit = {
+  def explain(mode: String): Unit = {
+    // Because temporary views are resolved during analysis when we create a Dataset, and
+    // `ExplainCommand` analyzes input query plan and resolves temporary views again. Using
+    // `ExplainCommand` here will probably output different query plans, compared to the results
+    // of evaluation of the Dataset. So just output QueryExecution's query plans here.
+
     // scalastyle:off println
-    println(toExplainString(mode))
+    println(queryExecution.explainString(ExplainMode.fromString(mode)))
 
 Review comment:
   I'm a little confused. This seems to be different from the original logic `ExplainCommandUtil.explainedQueryExecution(sparkSession, logicalPlan, queryExecution)` in case of  `logicalPlan.isStreaming == true`? Did I miss something?

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


With regards,
Apache Git Services

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