This is an automated email from the ASF dual-hosted git repository.
wenchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 5a3f41a [SPARK-34976][SQL] Rename GroupingSet to BaseGroupingSets
5a3f41a is described below
commit 5a3f41a017bf0fb07cf242e5c6ba25fc231863c6
Author: Angerszhuuuu <angers....@gmail.com>
AuthorDate: Wed Apr 7 13:27:21 2021 +0000
[SPARK-34976][SQL] Rename GroupingSet to BaseGroupingSets
### What changes were proposed in this pull request?
Current trait `GroupingSet` is ambiguous, since `grouping set` in parser
level means one set of a group.
Rename this to `BaseGroupingSets` since cube/rollup is syntax sugar for
grouping sets.`
### Why are the changes needed?
Refactor class name
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Not need
Closes #32073 from AngersZhuuuu/SPARK-34976.
Authored-by: Angerszhuuuu <angers....@gmail.com>
Signed-off-by: Wenchen Fan <wenc...@databricks.com>
---
.../spark/sql/catalyst/analysis/Analyzer.scala | 10 ++++-----
.../spark/sql/catalyst/expressions/grouping.scala | 25 ++++++++++++----------
.../spark/sql/catalyst/parser/AstBuilder.scala | 3 ++-
.../analysis/ResolveGroupingAnalyticsSuite.scala | 4 ++--
4 files changed, 23 insertions(+), 19 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 bbb7662..f50c28a 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
@@ -599,7 +599,7 @@ class Analyzer(override val catalogManager: CatalogManager)
val aggForResolving = h.child match {
// For CUBE/ROLLUP expressions, to avoid resolving repeatedly, here we
delete them from
// groupingExpressions for condition resolving.
- case a @ Aggregate(Seq(gs: GroupingSet), _, _) =>
+ case a @ Aggregate(Seq(gs: BaseGroupingSets), _, _) =>
a.copy(groupingExpressions = gs.groupByExprs)
}
// Try resolving the condition of the filter as though it is in the
aggregate clause
@@ -610,7 +610,7 @@ class Analyzer(override val catalogManager: CatalogManager)
if (resolvedInfo.nonEmpty) {
val (extraAggExprs, resolvedHavingCond) = resolvedInfo.get
val newChild = h.child match {
- case Aggregate(Seq(gs: GroupingSet), aggregateExpressions, child) =>
+ case Aggregate(Seq(gs: BaseGroupingSets), aggregateExpressions,
child) =>
constructAggregate(
gs.selectedGroupByExprs, gs.groupByExprs,
aggregateExpressions ++ extraAggExprs, child)
@@ -636,14 +636,14 @@ class Analyzer(override val catalogManager:
CatalogManager)
// CUBE/ROLLUP/GROUPING SETS. This also replace grouping()/grouping_id()
in resolved
// Filter/Sort.
def apply(plan: LogicalPlan): LogicalPlan = plan resolveOperatorsDown {
- case h @ UnresolvedHaving(_, agg @ Aggregate(Seq(gs: GroupingSet),
aggregateExpressions, _))
- if agg.childrenResolved && (gs.children ++
aggregateExpressions).forall(_.resolved) =>
+ case h @ UnresolvedHaving(_, agg @ Aggregate(Seq(gs: BaseGroupingSets),
aggExprs, _))
+ if agg.childrenResolved && (gs.children ++
aggExprs).forall(_.resolved) =>
tryResolveHavingCondition(h)
case a if !a.childrenResolved => a // be sure all of the children are
resolved.
// Ensure group by expressions and aggregate expressions have been
resolved.
- case Aggregate(Seq(gs: GroupingSet), aggregateExpressions, child)
+ case Aggregate(Seq(gs: BaseGroupingSets), aggregateExpressions, child)
if (gs.children ++ aggregateExpressions).forall(_.resolved) =>
constructAggregate(gs.selectedGroupByExprs, gs.groupByExprs,
aggregateExpressions, child)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/grouping.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/grouping.scala
index 66cd140..bf28efa 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/grouping.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/grouping.scala
@@ -26,14 +26,14 @@ import org.apache.spark.sql.types._
/**
* A placeholder expression for cube/rollup, which will be replaced by analyzer
*/
-trait GroupingSet extends Expression with CodegenFallback {
+trait BaseGroupingSets extends Expression with CodegenFallback {
def groupingSets: Seq[Seq[Expression]]
def selectedGroupByExprs: Seq[Seq[Expression]]
def groupByExprs: Seq[Expression] = {
assert(children.forall(_.resolved),
- "Cannot call GroupingSet.groupByExprs before the children expressions
are all resolved.")
+ "Cannot call BaseGroupingSets.groupByExprs before the children
expressions are all resolved.")
children.foldLeft(Seq.empty[Expression]) { (result, currentExpr) =>
// Only unique expressions are included in the group by expressions and
is determined
// based on their semantic equality. Example. grouping sets ((a * b), (b
* a)) results
@@ -55,7 +55,7 @@ trait GroupingSet extends Expression with CodegenFallback {
override def eval(input: InternalRow): Any = throw new
UnsupportedOperationException
}
-object GroupingSet {
+object BaseGroupingSets {
/**
* 'GROUP BY a, b, c WITH ROLLUP'
* is equivalent to
@@ -106,34 +106,37 @@ object GroupingSet {
}
}
-case class Cube(groupingSetIndexes: Seq[Seq[Int]], children: Seq[Expression])
extends GroupingSet {
+case class Cube(
+ groupingSetIndexes: Seq[Seq[Int]],
+ children: Seq[Expression]) extends BaseGroupingSets {
override def groupingSets: Seq[Seq[Expression]] =
groupingSetIndexes.map(_.map(children))
- override def selectedGroupByExprs: Seq[Seq[Expression]] =
GroupingSet.cubeExprs(groupingSets)
+ override def selectedGroupByExprs: Seq[Seq[Expression]] =
BaseGroupingSets.cubeExprs(groupingSets)
}
object Cube {
def apply(groupingSets: Seq[Seq[Expression]]): Cube = {
- Cube(GroupingSet.computeGroupingSetIndexes(groupingSets),
groupingSets.flatten)
+ Cube(BaseGroupingSets.computeGroupingSetIndexes(groupingSets),
groupingSets.flatten)
}
}
case class Rollup(
groupingSetIndexes: Seq[Seq[Int]],
- children: Seq[Expression]) extends GroupingSet {
+ children: Seq[Expression]) extends BaseGroupingSets {
override def groupingSets: Seq[Seq[Expression]] =
groupingSetIndexes.map(_.map(children))
- override def selectedGroupByExprs: Seq[Seq[Expression]] =
GroupingSet.rollupExprs(groupingSets)
+ override def selectedGroupByExprs: Seq[Seq[Expression]] =
+ BaseGroupingSets.rollupExprs(groupingSets)
}
object Rollup {
def apply(groupingSets: Seq[Seq[Expression]]): Rollup = {
- Rollup(GroupingSet.computeGroupingSetIndexes(groupingSets),
groupingSets.flatten)
+ Rollup(BaseGroupingSets.computeGroupingSetIndexes(groupingSets),
groupingSets.flatten)
}
}
case class GroupingSets(
groupingSetIndexes: Seq[Seq[Int]],
flatGroupingSets: Seq[Expression],
- userGivenGroupByExprs: Seq[Expression]) extends GroupingSet {
+ userGivenGroupByExprs: Seq[Expression]) extends BaseGroupingSets {
override def groupingSets: Seq[Seq[Expression]] =
groupingSetIndexes.map(_.map(flatGroupingSets))
override def selectedGroupByExprs: Seq[Seq[Expression]] = groupingSets
// Includes the `userGivenGroupByExprs` in the children, which will be
included in the final
@@ -145,7 +148,7 @@ object GroupingSets {
def apply(
groupingSets: Seq[Seq[Expression]],
userGivenGroupByExprs: Seq[Expression]): GroupingSets = {
- val groupingSetIndexes =
GroupingSet.computeGroupingSetIndexes(groupingSets)
+ val groupingSetIndexes =
BaseGroupingSets.computeGroupingSetIndexes(groupingSets)
GroupingSets(groupingSetIndexes, groupingSets.flatten,
userGivenGroupByExprs)
}
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
index fad5966..e9052fd 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
@@ -966,7 +966,8 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with
SQLConfHelper with Logg
expression(groupByExpr.expression)
}
})
- val (groupingSet, expressions) =
groupByExpressions.partition(_.isInstanceOf[GroupingSet])
+ val (groupingSet, expressions) =
+ groupByExpressions.partition(_.isInstanceOf[BaseGroupingSets])
if (expressions.nonEmpty && groupingSet.nonEmpty) {
throw new ParseException("Partial CUBE/ROLLUP/GROUPING SETS like " +
"`GROUP BY a, b, CUBE(a, b)` is not supported.",
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/ResolveGroupingAnalyticsSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/ResolveGroupingAnalyticsSuite.scala
index 1622928..ae36ab9 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/ResolveGroupingAnalyticsSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/ResolveGroupingAnalyticsSuite.scala
@@ -42,7 +42,7 @@ class ResolveGroupingAnalyticsSuite extends AnalysisTest {
test("rollupExprs") {
val testRollup = (exprs: Seq[Expression], rollup: Seq[Seq[Expression]]) =>
{
- val result = GroupingSet.rollupExprs(exprs.map(Seq(_)))
+ val result = BaseGroupingSets.rollupExprs(exprs.map(Seq(_)))
assert(result.sortBy(_.hashCode) == rollup.sortBy(_.hashCode))
}
@@ -54,7 +54,7 @@ class ResolveGroupingAnalyticsSuite extends AnalysisTest {
test("cubeExprs") {
val testCube = (exprs: Seq[Expression], cube: Seq[Seq[Expression]]) => {
- val result = GroupingSet.cubeExprs(exprs.map(Seq(_)))
+ val result = BaseGroupingSets.cubeExprs(exprs.map(Seq(_)))
assert(result.sortBy(_.hashCode) == cube.sortBy(_.hashCode))
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
