-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-17972][SQL] Add Dataset.checkpoint() to truncate large query plans #15651
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 all commits
609fba7
cee16ce
5405a94
ffe4318
e0e38bf
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 |
|---|---|---|
|
|
@@ -23,7 +23,7 @@ import org.apache.spark.sql.catalyst.{CatalystTypeConverters, InternalRow} | |
| import org.apache.spark.sql.catalyst.analysis.MultiInstanceRelation | ||
| import org.apache.spark.sql.catalyst.expressions._ | ||
| import org.apache.spark.sql.catalyst.plans.logical._ | ||
| import org.apache.spark.sql.execution.datasources._ | ||
| import org.apache.spark.sql.catalyst.plans.physical.{Partitioning, UnknownPartitioning} | ||
| import org.apache.spark.sql.execution.metric.SQLMetrics | ||
| import org.apache.spark.sql.types.DataType | ||
| import org.apache.spark.util.Utils | ||
|
|
@@ -130,17 +130,40 @@ case class ExternalRDDScanExec[T]( | |
| /** Logical plan node for scanning data from an RDD of InternalRow. */ | ||
| case class LogicalRDD( | ||
| output: Seq[Attribute], | ||
| rdd: RDD[InternalRow])(session: SparkSession) | ||
| rdd: RDD[InternalRow], | ||
| outputPartitioning: Partitioning = UnknownPartitioning(0), | ||
| outputOrdering: Seq[SortOrder] = Nil)(session: SparkSession) | ||
| extends LeafNode with MultiInstanceRelation { | ||
|
|
||
| override protected final def otherCopyArgs: Seq[AnyRef] = session :: Nil | ||
|
|
||
| override def newInstance(): LogicalRDD.this.type = | ||
| LogicalRDD(output.map(_.newInstance()), rdd)(session).asInstanceOf[this.type] | ||
| override def newInstance(): LogicalRDD.this.type = { | ||
| val rewrite = output.zip(output.map(_.newInstance())).toMap | ||
|
|
||
| val rewrittenPartitioning = outputPartitioning match { | ||
| case p: Expression => | ||
| p.transform { | ||
| case e: Attribute => rewrite.getOrElse(e, e) | ||
| }.asInstanceOf[Partitioning] | ||
|
|
||
| case p => p | ||
|
Contributor
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 explain this?
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. Not all |
||
| } | ||
|
|
||
| val rewrittenOrdering = outputOrdering.map(_.transform { | ||
| case e: Attribute => rewrite.getOrElse(e, e) | ||
| }.asInstanceOf[SortOrder]) | ||
|
|
||
| LogicalRDD( | ||
| output.map(rewrite), | ||
| rdd, | ||
| rewrittenPartitioning, | ||
| rewrittenOrdering | ||
| )(session).asInstanceOf[this.type] | ||
| } | ||
|
|
||
| override def sameResult(plan: LogicalPlan): Boolean = { | ||
| plan.canonicalized match { | ||
| case LogicalRDD(_, otherRDD) => rdd.id == otherRDD.id | ||
| case LogicalRDD(_, otherRDD, _, _) => rdd.id == otherRDD.id | ||
| case _ => false | ||
| } | ||
| } | ||
|
|
@@ -158,7 +181,9 @@ case class LogicalRDD( | |
| case class RDDScanExec( | ||
| output: Seq[Attribute], | ||
| rdd: RDD[InternalRow], | ||
| override val nodeName: String) extends LeafExecNode { | ||
| override val nodeName: String, | ||
| override val outputPartitioning: Partitioning = UnknownPartitioning(0), | ||
| override val outputOrdering: Seq[SortOrder] = Nil) extends LeafExecNode { | ||
|
|
||
| override lazy val metrics = Map( | ||
| "numOutputRows" -> SQLMetrics.createMetric(sparkContext, "number of output rows")) | ||
|
|
||
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.
The reason why we would like to pick the first leaf
Partitioninghere is thatPartitioningCollection, which is also anExpressionand participates query planning, may grow exponentially in the benchmark snippet, which essentially builds a full binary tree ofJoins.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.
Are the partitioning other than the first useful? Can we just filter out the partitioning guaranteed by other partitionings instead of picking the first only?
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.
There can be cases where the optimizer fails to eliminate an unnecessary shuffle if we strip all the other partitionings. But that's still better than an exponentially growing
PartitioningCollection, which basically runs into the same slow query planning issue this PR tries to solve.I talked to @yhuai offline about exactly the same issue you brought up before sending out this PR, and we decided to have a working version first and optimize it later since we still need feedback from ML people to see whether the basic mechanism works for their workloads.