From 043ba63ad9f3e7b006a73f3d2af61d7cb6ebbe24 Mon Sep 17 00:00:00 2001 From: gatorsmile Date: Thu, 17 Sep 2020 20:56:01 -0700 Subject: [PATCH 1/4] fix --- .../spark/sql/catalyst/analysis/unresolved.scala | 5 ----- .../spark/sql/catalyst/expressions/Expression.scala | 4 +++- .../spark/sql/catalyst/expressions/SortOrder.scala | 3 --- .../catalyst/expressions/aggregate/interfaces.scala | 1 - .../catalyst/expressions/complexTypeCreator.scala | 1 - .../spark/sql/catalyst/expressions/misc.scala | 2 -- .../spark/sql/catalyst/expressions/predicates.scala | 1 - .../catalyst/expressions/windowExpressions.scala | 13 ------------- .../sql/catalyst/plans/logical/v2Commands.scala | 2 -- 9 files changed, 3 insertions(+), 29 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala index 62000ac0efbb..581ffafa361c 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala @@ -255,7 +255,6 @@ case class UnresolvedFunction( override def children: Seq[Expression] = arguments ++ filter.toSeq override def dataType: DataType = throw new UnresolvedException(this, "dataType") - override def foldable: Boolean = throw new UnresolvedException(this, "foldable") override def nullable: Boolean = throw new UnresolvedException(this, "nullable") override lazy val resolved = false @@ -443,7 +442,6 @@ case class UnresolvedExtractValue(child: Expression, extraction: Expression) override def right: Expression = extraction override def dataType: DataType = throw new UnresolvedException(this, "dataType") - override def foldable: Boolean = throw new UnresolvedException(this, "foldable") override def nullable: Boolean = throw new UnresolvedException(this, "nullable") override lazy val resolved = false @@ -513,14 +511,12 @@ case class UnresolvedDeserializer(deserializer: Expression, inputAttributes: Seq override def child: Expression = deserializer override def dataType: DataType = throw new UnresolvedException(this, "dataType") - override def foldable: Boolean = throw new UnresolvedException(this, "foldable") override def nullable: Boolean = throw new UnresolvedException(this, "nullable") override lazy val resolved = false } case class GetColumnByOrdinal(ordinal: Int, dataType: DataType) extends LeafExpression with Unevaluable with NonSQLExpression { - override def foldable: Boolean = throw new UnresolvedException(this, "foldable") override def nullable: Boolean = throw new UnresolvedException(this, "nullable") override lazy val resolved = false } @@ -538,7 +534,6 @@ case class GetColumnByOrdinal(ordinal: Int, dataType: DataType) extends LeafExpr case class UnresolvedOrdinal(ordinal: Int) extends LeafExpression with Unevaluable with NonSQLExpression { override def dataType: DataType = throw new UnresolvedException(this, "dataType") - override def foldable: Boolean = throw new UnresolvedException(this, "foldable") override def nullable: Boolean = throw new UnresolvedException(this, "nullable") override lazy val resolved = false } diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala index 18cc648e57d7..ce4aa1c2b7c2 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala @@ -298,6 +298,9 @@ abstract class Expression extends TreeNode[Expression] { */ trait Unevaluable extends Expression { + /** Unevaluable is not foldable because we don't have an eval for it. */ + final override def foldable: Boolean = false + final override def eval(input: InternalRow = null): Any = throw new UnsupportedOperationException(s"Cannot evaluate expression: $this") @@ -318,7 +321,6 @@ trait Unevaluable extends Expression { */ trait RuntimeReplaceable extends UnaryExpression with Unevaluable { override def nullable: Boolean = child.nullable - override def foldable: Boolean = child.foldable override def dataType: DataType = child.dataType // As this expression gets replaced at optimization with its `child" expression, // two `RuntimeReplaceable` are considered to be semantically equal if their "child" expressions diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala index 536276b5cb29..54259e713acc 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala @@ -66,9 +66,6 @@ case class SortOrder( sameOrderExpressions: Set[Expression]) extends UnaryExpression with Unevaluable { - /** Sort order is not foldable because we don't have an eval for it. */ - override def foldable: Boolean = false - override def checkInputDataTypes(): TypeCheckResult = { if (RowOrdering.isOrderable(dataType)) { TypeCheckResult.TypeCheckSuccess diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/interfaces.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/interfaces.scala index 26367cc058bf..2a64670dd331 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/interfaces.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/interfaces.scala @@ -133,7 +133,6 @@ case class AggregateExpression( override def children: Seq[Expression] = aggregateFunction +: filter.toSeq override def dataType: DataType = aggregateFunction.dataType - override def foldable: Boolean = false override def nullable: Boolean = aggregateFunction.nullable @transient diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala index 563ce7133a3d..36236c8e9c8f 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala @@ -301,7 +301,6 @@ case class MapFromArrays(left: Expression, right: Expression) */ case object NamePlaceholder extends LeafExpression with Unevaluable { override lazy val resolved: Boolean = false - override def foldable: Boolean = false override def nullable: Boolean = false override def dataType: DataType = StringType override def prettyName: String = "NamePlaceholder" diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala index 617ddcb69eab..a286a9c294f6 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala @@ -111,7 +111,6 @@ case class AssertTrue(child: Expression) extends UnaryExpression with ImplicitCa """) case class CurrentDatabase() extends LeafExpression with Unevaluable { override def dataType: DataType = StringType - override def foldable: Boolean = true override def nullable: Boolean = false override def prettyName: String = "current_database" } @@ -129,7 +128,6 @@ case class CurrentDatabase() extends LeafExpression with Unevaluable { since = "3.1.0") case class CurrentCatalog() extends LeafExpression with Unevaluable { override def dataType: DataType = StringType - override def foldable: Boolean = true override def nullable: Boolean = false override def prettyName: String = "current_catalog" } diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala index 03066fb34cf2..06a11f763f6c 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala @@ -327,7 +327,6 @@ case class InSubquery(values: Seq[Expression], query: ListQuery) override def children: Seq[Expression] = values :+ query override def nullable: Boolean = children.exists(_.nullable) - override def foldable: Boolean = children.forall(_.foldable) override def toString: String = s"$value IN ($query)" override def sql: String = s"(${value.sql} IN (${query.sql}))" } diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala index c8b643320735..6f9ccbd39537 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala @@ -50,7 +50,6 @@ case class WindowSpecDefinition( frameSpecification.isInstanceOf[SpecifiedWindowFrame] override def nullable: Boolean = true - override def foldable: Boolean = false override def dataType: DataType = throw new UnsupportedOperationException("dataType") override def checkInputDataTypes(): TypeCheckResult = { @@ -144,7 +143,6 @@ case object RangeFrame extends FrameType { sealed trait SpecialFrameBoundary extends Expression with Unevaluable { override def children: Seq[Expression] = Nil override def dataType: DataType = NullType - override def foldable: Boolean = false override def nullable: Boolean = false } @@ -168,7 +166,6 @@ case object CurrentRow extends SpecialFrameBoundary { sealed trait WindowFrame extends Expression with Unevaluable { override def children: Seq[Expression] = Nil override def dataType: DataType = throw new UnsupportedOperationException("dataType") - override def foldable: Boolean = false override def nullable: Boolean = false } @@ -275,7 +272,6 @@ case class UnresolvedWindowExpression( windowSpec: WindowSpecReference) extends UnaryExpression with Unevaluable { override def dataType: DataType = throw new UnresolvedException(this, "dataType") - override def foldable: Boolean = throw new UnresolvedException(this, "foldable") override def nullable: Boolean = throw new UnresolvedException(this, "nullable") override lazy val resolved = false } @@ -287,7 +283,6 @@ case class WindowExpression( override def children: Seq[Expression] = windowFunction :: windowSpec :: Nil override def dataType: DataType = windowFunction.dataType - override def foldable: Boolean = windowFunction.foldable override def nullable: Boolean = windowFunction.nullable override def toString: String = s"$windowFunction $windowSpec" @@ -365,14 +360,6 @@ abstract class OffsetWindowFunction override def children: Seq[Expression] = Seq(input, offset, default) - /* - * The result of an OffsetWindowFunction is dependent on the frame in which the - * OffsetWindowFunction is executed, the input expression and the default expression. Even when - * both the input and the default expression are foldable, the result is still not foldable due to - * the frame. - */ - override def foldable: Boolean = false - override def nullable: Boolean = default == null || default.nullable || input.nullable override lazy val frame: WindowFrame = { diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala index 70e03c23fd11..3b51761abde4 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala @@ -348,7 +348,6 @@ case class MergeIntoTable( sealed abstract class MergeAction extends Expression with Unevaluable { def condition: Option[Expression] - override def foldable: Boolean = false override def nullable: Boolean = false override def dataType: DataType = throw new UnresolvedException(this, "nullable") override def children: Seq[Expression] = condition.toSeq @@ -369,7 +368,6 @@ case class InsertAction( } case class Assignment(key: Expression, value: Expression) extends Expression with Unevaluable { - override def foldable: Boolean = false override def nullable: Boolean = false override def dataType: DataType = throw new UnresolvedException(this, "nullable") override def children: Seq[Expression] = key :: value :: Nil From ca4b37193a1dfc7ecfd4ff6a20f866cc02322f23 Mon Sep 17 00:00:00 2001 From: gatorsmile Date: Thu, 17 Sep 2020 22:50:57 -0700 Subject: [PATCH 2/4] fix --- .../catalyst/expressions/aggregate/interfaces.scala | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/interfaces.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/interfaces.scala index 2a64670dd331..421b8ee2a25b 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/interfaces.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/interfaces.scala @@ -20,7 +20,7 @@ package org.apache.spark.sql.catalyst.expressions.aggregate import org.apache.spark.sql.catalyst.InternalRow import org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute import org.apache.spark.sql.catalyst.expressions._ -import org.apache.spark.sql.catalyst.expressions.codegen.CodegenFallback +import org.apache.spark.sql.catalyst.expressions.codegen.{CodegenContext, CodegenFallback, ExprCode} import org.apache.spark.sql.types._ /** The mode of an [[AggregateFunction]]. */ @@ -373,8 +373,7 @@ abstract class ImperativeAggregate extends AggregateFunction with CodegenFallbac */ abstract class DeclarativeAggregate extends AggregateFunction - with Serializable - with Unevaluable { + with Serializable { /** * Expressions for initializing empty aggregation buffers. @@ -420,6 +419,12 @@ abstract class DeclarativeAggregate /** Represents this attribute at the input buffer side (the data value is read-only). */ def right: AttributeReference = inputAggBufferAttributes(aggBufferAttributes.indexOf(a)) } + + final override def eval(input: InternalRow = null): Any = + throw new UnsupportedOperationException(s"Cannot evaluate expression: $this") + + final override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = + throw new UnsupportedOperationException(s"Cannot generate code for expression: $this") } From 590eef55d0608f2537f9ecbf4ca6cd140cca0967 Mon Sep 17 00:00:00 2001 From: Xiao Li Date: Tue, 22 Sep 2020 22:55:01 -0700 Subject: [PATCH 3/4] address comments --- .../sql/catalyst/expressions/windowExpressions.scala | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala index 6f9ccbd39537..382ac33b421d 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala @@ -45,6 +45,17 @@ case class WindowSpecDefinition( override def children: Seq[Expression] = partitionSpec ++ orderSpec :+ frameSpecification + /* + * The result of an OffsetWindowFunction is dependent on the frame in which the + * OffsetWindowFunction is executed, the input expression and the default expression. Even when + * both the input and the default expression are foldable, the result is still not foldable due to + * the frame. + * + * Note, the value of foldable is set to false in the trait Unevaluable + * + * override def foldable: Boolean = false + */ + override lazy val resolved: Boolean = childrenResolved && checkInputDataTypes().isSuccess && frameSpecification.isInstanceOf[SpecifiedWindowFrame] From 7ecb1e6987f87b7b1e3164ef227cc6fb45243a23 Mon Sep 17 00:00:00 2001 From: Xiao Li Date: Wed, 23 Sep 2020 07:40:11 -0700 Subject: [PATCH 4/4] fix --- .../expressions/windowExpressions.scala | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala index 382ac33b421d..e9e7b071a26f 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala @@ -45,17 +45,6 @@ case class WindowSpecDefinition( override def children: Seq[Expression] = partitionSpec ++ orderSpec :+ frameSpecification - /* - * The result of an OffsetWindowFunction is dependent on the frame in which the - * OffsetWindowFunction is executed, the input expression and the default expression. Even when - * both the input and the default expression are foldable, the result is still not foldable due to - * the frame. - * - * Note, the value of foldable is set to false in the trait Unevaluable - * - * override def foldable: Boolean = false - */ - override lazy val resolved: Boolean = childrenResolved && checkInputDataTypes().isSuccess && frameSpecification.isInstanceOf[SpecifiedWindowFrame] @@ -371,6 +360,17 @@ abstract class OffsetWindowFunction override def children: Seq[Expression] = Seq(input, offset, default) + /* + * The result of an OffsetWindowFunction is dependent on the frame in which the + * OffsetWindowFunction is executed, the input expression and the default expression. Even when + * both the input and the default expression are foldable, the result is still not foldable due to + * the frame. + * + * Note, the value of foldable is set to false in the trait Unevaluable + * + * override def foldable: Boolean = false + */ + override def nullable: Boolean = default == null || default.nullable || input.nullable override lazy val frame: WindowFrame = {