Repository: spark Updated Branches: refs/heads/branch-2.0 1ff738afc -> a5c178bc0
Revert "[SPARK-18854][SQL] numberedTreeString and apply(i) inconsistent for subqueries" This reverts commit 1ff738afc1b11eacb11ac4f37324334a6b6fe41b. Project: http://git-wip-us.apache.org/repos/asf/spark/repo Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/a5c178bc Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/a5c178bc Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/a5c178bc Branch: refs/heads/branch-2.0 Commit: a5c178bc07092b698ee17894a439deb47699db0f Parents: 1ff738a Author: Reynold Xin <[email protected]> Authored: Wed Dec 14 16:30:47 2016 -0800 Committer: Reynold Xin <[email protected]> Committed: Wed Dec 14 16:30:47 2016 -0800 ---------------------------------------------------------------------- .../spark/sql/catalyst/plans/QueryPlan.scala | 9 ---- .../spark/sql/catalyst/trees/TreeNode.scala | 43 ++++++++------------ .../execution/columnar/InMemoryRelation.scala | 3 +- .../org/apache/spark/sql/SubquerySuite.scala | 18 -------- 4 files changed, 19 insertions(+), 54 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/spark/blob/a5c178bc/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala ---------------------------------------------------------------------- diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala index bcc7521..41c4e00 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala @@ -24,15 +24,6 @@ import org.apache.spark.sql.types.{DataType, StructType} abstract class QueryPlan[PlanType <: QueryPlan[PlanType]] extends TreeNode[PlanType] { self: PlanType => - /** - * Override [[TreeNode.apply]] to so we can return a more narrow type. - * - * Note that this cannot return BaseType because logical plan's plan node might return - * physical plan for innerChildren, e.g. in-memory relation logical plan node has a reference - * to the physical plan node it is referencing. - */ - override def apply(number: Int): QueryPlan[_] = super.apply(number).asInstanceOf[QueryPlan[_]] - def output: Seq[Attribute] /** http://git-wip-us.apache.org/repos/asf/spark/blob/a5c178bc/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala ---------------------------------------------------------------------- diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala index 877a16d..931d14d 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala @@ -20,6 +20,7 @@ package org.apache.spark.sql.catalyst.trees import java.util.UUID import scala.collection.Map +import scala.collection.mutable.Stack import scala.reflect.ClassTag import org.apache.commons.lang3.ClassUtils @@ -482,10 +483,7 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] extends Product { /** * Returns a string representation of the nodes in this tree, where each operator is numbered. - * The numbers can be used with [[TreeNode.apply]] to easily access specific subtrees. - * - * The numbers are based on depth-first traversal of the tree (with innerChildren traversed first - * before children). + * The numbers can be used with [[trees.TreeNode.apply apply]] to easily access specific subtrees. */ def numberedTreeString: String = treeString.split("\n").zipWithIndex.map { case (line, i) => f"$i%02d $line" }.mkString("\n") @@ -493,24 +491,17 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] extends Product { /** * Returns the tree node at the specified number. * Numbers for each node can be found in the [[numberedTreeString]]. - * - * Note that this cannot return BaseType because logical plan's plan node might return - * physical plan for innerChildren, e.g. in-memory relation logical plan node has a reference - * to the physical plan node it is referencing. */ - def apply(number: Int): TreeNode[_] = getNodeNumbered(new MutableInt(number)).orNull + def apply(number: Int): BaseType = getNodeNumbered(new MutableInt(number)) - private def getNodeNumbered(number: MutableInt): Option[TreeNode[_]] = { + protected def getNodeNumbered(number: MutableInt): BaseType = { if (number.i < 0) { - None + null.asInstanceOf[BaseType] } else if (number.i == 0) { - Some(this) + this } else { number.i -= 1 - // Note that this traversal order must be the same as numberedTreeString. - innerChildren.map(_.getNodeNumbered(number)).find(_ != None).getOrElse { - children.map(_.getNodeNumbered(number)).find(_ != None).flatten - } + children.map(_.getNodeNumbered(number)).find(_ != null).getOrElse(null.asInstanceOf[BaseType]) } } @@ -526,8 +517,6 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] extends Product { * The `i`-th element in `lastChildren` indicates whether the ancestor of the current node at * depth `i + 1` is the last child of its own parent node. The depth of the root node is 0, and * `lastChildren` for the root node should be empty. - * - * Note that this traversal (numbering) order must be the same as [[getNodeNumbered]]. */ def generateTreeString( depth: Int, @@ -535,16 +524,19 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] extends Product { builder: StringBuilder, verbose: Boolean, prefix: String = ""): StringBuilder = { - if (depth > 0) { lastChildren.init.foreach { isLast => - builder.append(if (isLast) " " else ": ") + val prefixFragment = if (isLast) " " else ": " + builder.append(prefixFragment) } - builder.append(if (lastChildren.last) "+- " else ":- ") + + val branch = if (lastChildren.last) "+- " else ":- " + builder.append(branch) } builder.append(prefix) - builder.append(if (verbose) verboseString else simpleString) + val headline = if (verbose) verboseString else simpleString + builder.append(headline) builder.append("\n") if (innerChildren.nonEmpty) { @@ -555,10 +547,9 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] extends Product { } if (children.nonEmpty) { - children.init.foreach(_.generateTreeString( - depth + 1, lastChildren :+ false, builder, verbose, prefix)) - children.last.generateTreeString( - depth + 1, lastChildren :+ true, builder, verbose, prefix) + children.init.foreach( + _.generateTreeString(depth + 1, lastChildren :+ false, builder, verbose, prefix)) + children.last.generateTreeString(depth + 1, lastChildren :+ true, builder, verbose, prefix) } builder http://git-wip-us.apache.org/repos/asf/spark/blob/a5c178bc/sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala ---------------------------------------------------------------------- diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala index 03cc046..56bd5c1 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala @@ -24,6 +24,7 @@ import org.apache.spark.rdd.RDD import org.apache.spark.sql.catalyst.InternalRow import org.apache.spark.sql.catalyst.analysis.MultiInstanceRelation import org.apache.spark.sql.catalyst.expressions._ +import org.apache.spark.sql.catalyst.plans.QueryPlan import org.apache.spark.sql.catalyst.plans.logical import org.apache.spark.sql.catalyst.plans.logical.Statistics import org.apache.spark.sql.execution.SparkPlan @@ -63,7 +64,7 @@ case class InMemoryRelation( val batchStats: LongAccumulator = child.sqlContext.sparkContext.longAccumulator) extends logical.LeafNode with MultiInstanceRelation { - override protected def innerChildren: Seq[SparkPlan] = Seq(child) + override protected def innerChildren: Seq[QueryPlan[_]] = Seq(child) override def producedAttributes: AttributeSet = outputSet http://git-wip-us.apache.org/repos/asf/spark/blob/a5c178bc/sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala ---------------------------------------------------------------------- diff --git a/sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala index eff8894..b145c69 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala @@ -54,24 +54,6 @@ class SubquerySuite extends QueryTest with SharedSQLContext { t.createOrReplaceTempView("t") } - test("SPARK-18854 numberedTreeString for subquery") { - val df = sql("select * from range(10) where id not in " + - "(select id from range(2) union all select id from range(2))") - - // The depth first traversal of the plan tree - val dfs = Seq("Project", "Filter", "Union", "Project", "Range", "Project", "Range", "Range") - val numbered = df.queryExecution.analyzed.numberedTreeString.split("\n") - - // There should be 8 plan nodes in total - assert(numbered.size == dfs.size) - - for (i <- dfs.indices) { - val node = df.queryExecution.analyzed(i) - assert(node.nodeName == dfs(i)) - assert(numbered(i).contains(node.nodeName)) - } - } - test("rdd deserialization does not crash [SPARK-15791]") { sql("select (select 1 as b) as b").rdd.count() } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
