diff options
author | petermaxlee <petermaxlee@gmail.com> | 2016-08-18 16:17:01 +0800 |
---|---|---|
committer | Wenchen Fan <wenchen@databricks.com> | 2016-08-18 16:17:01 +0800 |
commit | 3e6ef2e8a435a91b6a76876e9833917e5aa0945e (patch) | |
tree | 97f99b03e7c5c4329086c2ad0380c805d3e9010b /sql/catalyst/src/main | |
parent | 10204b9d29cd69895f5a606e75510dc64cf2e009 (diff) | |
download | spark-3e6ef2e8a435a91b6a76876e9833917e5aa0945e.tar.gz spark-3e6ef2e8a435a91b6a76876e9833917e5aa0945e.tar.bz2 spark-3e6ef2e8a435a91b6a76876e9833917e5aa0945e.zip |
[SPARK-17034][SQL] Minor code cleanup for UnresolvedOrdinal
## What changes were proposed in this pull request?
I was looking at the code for UnresolvedOrdinal and made a few small changes to make it slightly more clear:
1. Rename the rule to SubstituteUnresolvedOrdinals which is more consistent with other rules that start with verbs. Note that this is still inconsistent with CTESubstitution and WindowsSubstitution.
2. Broke the test suite down from a single test case to three test cases.
## How was this patch tested?
This is a minor cleanup.
Author: petermaxlee <petermaxlee@gmail.com>
Closes #14672 from petermaxlee/SPARK-17034.
Diffstat (limited to 'sql/catalyst/src/main')
-rw-r--r-- | sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala | 2 | ||||
-rw-r--r-- | sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/SubstituteUnresolvedOrdinals.scala (renamed from sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/UnresolvedOrdinalSubstitution.scala) | 26 | ||||
-rw-r--r-- | sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/planning/patterns.scala | 13 |
3 files changed, 15 insertions, 26 deletions
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala index f540816366..cfab6ae7bd 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala @@ -84,7 +84,7 @@ class Analyzer( CTESubstitution, WindowsSubstitution, EliminateUnions, - new UnresolvedOrdinalSubstitution(conf)), + new SubstituteUnresolvedOrdinals(conf)), Batch("Resolution", fixedPoint, ResolveRelations :: ResolveReferences :: diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/UnresolvedOrdinalSubstitution.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/SubstituteUnresolvedOrdinals.scala index e21cd08af8..6d8dc86282 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/UnresolvedOrdinalSubstitution.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/SubstituteUnresolvedOrdinals.scala @@ -18,32 +18,34 @@ package org.apache.spark.sql.catalyst.analysis import org.apache.spark.sql.catalyst.CatalystConf -import org.apache.spark.sql.catalyst.expressions.{Expression, SortOrder} -import org.apache.spark.sql.catalyst.planning.IntegerIndex +import org.apache.spark.sql.catalyst.expressions.{Expression, Literal, SortOrder} import org.apache.spark.sql.catalyst.plans.logical.{Aggregate, LogicalPlan, Sort} import org.apache.spark.sql.catalyst.rules.Rule import org.apache.spark.sql.catalyst.trees.CurrentOrigin.withOrigin +import org.apache.spark.sql.types.IntegerType /** * Replaces ordinal in 'order by' or 'group by' with UnresolvedOrdinal expression. */ -class UnresolvedOrdinalSubstitution(conf: CatalystConf) extends Rule[LogicalPlan] { - private def isIntegerLiteral(sorter: Expression) = IntegerIndex.unapply(sorter).nonEmpty +class SubstituteUnresolvedOrdinals(conf: CatalystConf) extends Rule[LogicalPlan] { + private def isIntLiteral(e: Expression) = e match { + case Literal(_, IntegerType) => true + case _ => false + } def apply(plan: LogicalPlan): LogicalPlan = plan transform { - case s @ Sort(orders, global, child) if conf.orderByOrdinal && - orders.exists(o => isIntegerLiteral(o.child)) => - val newOrders = orders.map { - case order @ SortOrder(ordinal @ IntegerIndex(index: Int), _) => + case s: Sort if conf.orderByOrdinal && s.order.exists(o => isIntLiteral(o.child)) => + val newOrders = s.order.map { + case order @ SortOrder(ordinal @ Literal(index: Int, IntegerType), _) => val newOrdinal = withOrigin(ordinal.origin)(UnresolvedOrdinal(index)) withOrigin(order.origin)(order.copy(child = newOrdinal)) case other => other } withOrigin(s.origin)(s.copy(order = newOrders)) - case a @ Aggregate(groups, aggs, child) if conf.groupByOrdinal && - groups.exists(isIntegerLiteral(_)) => - val newGroups = groups.map { - case ordinal @ IntegerIndex(index) => + + case a: Aggregate if conf.groupByOrdinal && a.groupingExpressions.exists(isIntLiteral) => + val newGroups = a.groupingExpressions.map { + case ordinal @ Literal(index: Int, IntegerType) => withOrigin(ordinal.origin)(UnresolvedOrdinal(index)) case other => other } diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/planning/patterns.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/planning/patterns.scala index f42e67ca6e..476c66af76 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/planning/patterns.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/planning/patterns.scala @@ -209,19 +209,6 @@ object Unions { } /** - * Extractor for retrieving Int value. - */ -object IntegerIndex { - def unapply(a: Any): Option[Int] = a match { - case Literal(a: Int, IntegerType) => Some(a) - // When resolving ordinal in Sort and Group By, negative values are extracted - // for issuing error messages. - case UnaryMinus(IntegerLiteral(v)) => Some(-v) - case _ => None - } -} - -/** * An extractor used when planning the physical execution of an aggregation. Compared with a logical * aggregation, the following transformations are performed: * - Unnamed grouping expressions are named so that they can be referred to across phases of |