-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-17078] [SQL] Show stats when explain #16594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
e25e71e
d08f4e0
b7c3a13
18b8d54
9fe22a5
491ec8f
b3457a0
6e10f84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,13 +17,16 @@ | |
|
|
||
| package org.apache.spark.sql.catalyst.plans.logical | ||
|
|
||
| import java.math.{MathContext, RoundingMode} | ||
|
|
||
| import scala.util.control.NonFatal | ||
|
|
||
| import org.apache.spark.internal.Logging | ||
| import org.apache.spark.sql.{AnalysisException, Row} | ||
| import org.apache.spark.sql.catalyst.expressions._ | ||
| import org.apache.spark.sql.catalyst.expressions.aggregate._ | ||
| import org.apache.spark.sql.types._ | ||
| import org.apache.spark.util.Utils | ||
|
|
||
|
|
||
| /** | ||
|
|
@@ -54,11 +57,29 @@ case class Statistics( | |
|
|
||
| /** Readable string representation for the Statistics. */ | ||
| def simpleString: String = { | ||
| Seq(s"sizeInBytes=$sizeInBytes", | ||
| if (rowCount.isDefined) s"rowCount=${rowCount.get}" else "", | ||
| Seq(s"sizeInBytes=${format(sizeInBytes, isSize = true)}", | ||
| if (rowCount.isDefined) s"rowCount=${format(rowCount.get, isSize = false)}" else "", | ||
| s"isBroadcastable=$isBroadcastable" | ||
| ).filter(_.nonEmpty).mkString(", ") | ||
| } | ||
|
|
||
| /** Show the given number in a readable format. */ | ||
| def format(number: BigInt, isSize: Boolean): String = { | ||
| val decimalValue = BigDecimal(number, new MathContext(3, RoundingMode.HALF_UP)) | ||
| if (isSize) { | ||
| // The largest unit in Utils.bytesToString is TB | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about improving
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea, I also think TB is a little small |
||
| val PB = 1L << 50 | ||
| if (number < 2 * PB) { | ||
| // The number is not very large, so we can use Utils.bytesToString to show it. | ||
| Utils.bytesToString(number.toLong) | ||
| } else { | ||
| // The number is too large, show it in scientific notation. | ||
| decimalValue.toString() + " B" | ||
| } | ||
| } else { | ||
| decimalValue.toString() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Always represent it using scientific notation? Or only do it when the number is too large?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://en.wikipedia.org/wiki/Metric_prefix Even if we do not have a unit, we still can use K, M, G, T, P, E?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure, will that be more readable than scientific notation if no unit?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With or without units, the readability is the same, right? If we make them consistent, the impl of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't make them consistent here, because unit string is added inside |
||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -197,20 +197,32 @@ class QueryExecution(val sparkSession: SparkSession, val logical: LogicalPlan) { | |
| """.stripMargin.trim | ||
| } | ||
|
|
||
| override def toString: String = { | ||
| override def toString: String = completeString(appendStats = false) | ||
|
|
||
| def toStringWithStats: String = completeString(appendStats = true) | ||
|
|
||
| def completeString(appendStats: Boolean): String = { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. private? |
||
| def output = Utils.truncatedString( | ||
| analyzed.output.map(o => s"${o.name}: ${o.dataType.simpleString}"), ", ") | ||
| val analyzedPlan = Seq( | ||
| stringOrError(output), | ||
| stringOrError(analyzed.treeString(verbose = true)) | ||
| ).filter(_.nonEmpty).mkString("\n") | ||
|
|
||
| val optimizedPlanString = if (appendStats) { | ||
| // trigger to compute stats for logical plans | ||
| optimizedPlan.stats(sparkSession.sessionState.conf) | ||
| optimizedPlan.treeString(verbose = true, addSuffix = true) | ||
| } else { | ||
| optimizedPlan.treeString(verbose = true) | ||
| } | ||
|
|
||
| s"""== Parsed Logical Plan == | ||
| |${stringOrError(logical.treeString(verbose = true))} | ||
| |== Analyzed Logical Plan == | ||
| |$analyzedPlan | ||
| |== Optimized Logical Plan == | ||
| |${stringOrError(optimizedPlan.treeString(verbose = true))} | ||
| |${stringOrError(optimizedPlanString)} | ||
| |== Physical Plan == | ||
| |${stringOrError(executedPlan.treeString(verbose = true))} | ||
| """.stripMargin.trim | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -282,7 +282,8 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { | |
| if (statement == null) { | ||
| null // This is enough since ParseException will raise later. | ||
| } else if (isExplainableStatement(statement)) { | ||
| ExplainCommand(statement, extended = ctx.EXTENDED != null, codegen = ctx.CODEGEN != null) | ||
| ExplainCommand(statement, extended = ctx.EXTENDED != null, codegen = ctx.CODEGEN != null, | ||
| cost = ctx.COST != null) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to fix the style.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you give a clue on the style?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } else { | ||
| ExplainCommand(OneRowRelation) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -92,7 +92,8 @@ case class ExecutedCommandExec(cmd: RunnableCommand) extends SparkPlan { | |
| case class ExplainCommand( | ||
| logicalPlan: LogicalPlan, | ||
| extended: Boolean = false, | ||
| codegen: Boolean = false) | ||
| codegen: Boolean = false, | ||
| cost: Boolean = false) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add |
||
| extends RunnableCommand { | ||
|
|
||
| override val output: Seq[Attribute] = | ||
|
|
@@ -113,6 +114,8 @@ case class ExplainCommand( | |
| codegenString(queryExecution.executedPlan) | ||
| } else if (extended) { | ||
| queryExecution.toString | ||
| } else if (cost) { | ||
| queryExecution.toStringWithStats | ||
| } else { | ||
| queryExecution.simpleString | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also put in it
nonReservedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Also please update the
hiveNonReservedKeywordinTableIdentifierParserSuiteThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Updated.