Skip to content

Commit e887c63

Browse files
gatorsmilecloud-fan
authored andcommitted
[SPARK-32931][SQL] Unevaluable Expressions are not Foldable
### What changes were proposed in this pull request? Unevaluable expressions are not foldable because we don't have an eval for it. This PR is to clean up the code and enforce it. ### Why are the changes needed? Ensure that we will not hit the weird cases that trigger ConstantFolding. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? The existing tests. Closes #29798 from gatorsmile/refactorUneval. Lead-authored-by: gatorsmile <[email protected]> Co-authored-by: Xiao Li <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
1 parent 9e6882f commit e887c63

File tree

9 files changed

+15
-25
lines changed

9 files changed

+15
-25
lines changed

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,6 @@ case class UnresolvedFunction(
264264
override def children: Seq[Expression] = arguments ++ filter.toSeq
265265

266266
override def dataType: DataType = throw new UnresolvedException(this, "dataType")
267-
override def foldable: Boolean = throw new UnresolvedException(this, "foldable")
268267
override def nullable: Boolean = throw new UnresolvedException(this, "nullable")
269268
override lazy val resolved = false
270269

@@ -452,7 +451,6 @@ case class UnresolvedExtractValue(child: Expression, extraction: Expression)
452451
override def right: Expression = extraction
453452

454453
override def dataType: DataType = throw new UnresolvedException(this, "dataType")
455-
override def foldable: Boolean = throw new UnresolvedException(this, "foldable")
456454
override def nullable: Boolean = throw new UnresolvedException(this, "nullable")
457455
override lazy val resolved = false
458456

@@ -522,14 +520,12 @@ case class UnresolvedDeserializer(deserializer: Expression, inputAttributes: Seq
522520

523521
override def child: Expression = deserializer
524522
override def dataType: DataType = throw new UnresolvedException(this, "dataType")
525-
override def foldable: Boolean = throw new UnresolvedException(this, "foldable")
526523
override def nullable: Boolean = throw new UnresolvedException(this, "nullable")
527524
override lazy val resolved = false
528525
}
529526

530527
case class GetColumnByOrdinal(ordinal: Int, dataType: DataType) extends LeafExpression
531528
with Unevaluable with NonSQLExpression {
532-
override def foldable: Boolean = throw new UnresolvedException(this, "foldable")
533529
override def nullable: Boolean = throw new UnresolvedException(this, "nullable")
534530
override lazy val resolved = false
535531
}
@@ -547,7 +543,6 @@ case class GetColumnByOrdinal(ordinal: Int, dataType: DataType) extends LeafExpr
547543
case class UnresolvedOrdinal(ordinal: Int)
548544
extends LeafExpression with Unevaluable with NonSQLExpression {
549545
override def dataType: DataType = throw new UnresolvedException(this, "dataType")
550-
override def foldable: Boolean = throw new UnresolvedException(this, "foldable")
551546
override def nullable: Boolean = throw new UnresolvedException(this, "nullable")
552547
override lazy val resolved = false
553548
}

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,9 @@ abstract class Expression extends TreeNode[Expression] {
298298
*/
299299
trait Unevaluable extends Expression {
300300

301+
/** Unevaluable is not foldable because we don't have an eval for it. */
302+
final override def foldable: Boolean = false
303+
301304
final override def eval(input: InternalRow = null): Any =
302305
throw new UnsupportedOperationException(s"Cannot evaluate expression: $this")
303306

@@ -318,7 +321,6 @@ trait Unevaluable extends Expression {
318321
*/
319322
trait RuntimeReplaceable extends UnaryExpression with Unevaluable {
320323
override def nullable: Boolean = child.nullable
321-
override def foldable: Boolean = child.foldable
322324
override def dataType: DataType = child.dataType
323325
// As this expression gets replaced at optimization with its `child" expression,
324326
// two `RuntimeReplaceable` are considered to be semantically equal if their "child" expressions

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,6 @@ case class SortOrder(
6666
sameOrderExpressions: Set[Expression])
6767
extends UnaryExpression with Unevaluable {
6868

69-
/** Sort order is not foldable because we don't have an eval for it. */
70-
override def foldable: Boolean = false
71-
7269
override def checkInputDataTypes(): TypeCheckResult = {
7370
if (RowOrdering.isOrderable(dataType)) {
7471
TypeCheckResult.TypeCheckSuccess

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/interfaces.scala

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ package org.apache.spark.sql.catalyst.expressions.aggregate
2020
import org.apache.spark.sql.catalyst.InternalRow
2121
import org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute
2222
import org.apache.spark.sql.catalyst.expressions._
23-
import org.apache.spark.sql.catalyst.expressions.codegen.CodegenFallback
23+
import org.apache.spark.sql.catalyst.expressions.codegen.{CodegenContext, CodegenFallback, ExprCode}
2424
import org.apache.spark.sql.types._
2525

2626
/** The mode of an [[AggregateFunction]]. */
@@ -133,7 +133,6 @@ case class AggregateExpression(
133133
override def children: Seq[Expression] = aggregateFunction +: filter.toSeq
134134

135135
override def dataType: DataType = aggregateFunction.dataType
136-
override def foldable: Boolean = false
137136
override def nullable: Boolean = aggregateFunction.nullable
138137

139138
@transient
@@ -374,8 +373,7 @@ abstract class ImperativeAggregate extends AggregateFunction with CodegenFallbac
374373
*/
375374
abstract class DeclarativeAggregate
376375
extends AggregateFunction
377-
with Serializable
378-
with Unevaluable {
376+
with Serializable {
379377

380378
/**
381379
* Expressions for initializing empty aggregation buffers.
@@ -421,6 +419,12 @@ abstract class DeclarativeAggregate
421419
/** Represents this attribute at the input buffer side (the data value is read-only). */
422420
def right: AttributeReference = inputAggBufferAttributes(aggBufferAttributes.indexOf(a))
423421
}
422+
423+
final override def eval(input: InternalRow = null): Any =
424+
throw new UnsupportedOperationException(s"Cannot evaluate expression: $this")
425+
426+
final override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode =
427+
throw new UnsupportedOperationException(s"Cannot generate code for expression: $this")
424428
}
425429

426430

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,6 @@ case class MapFromArrays(left: Expression, right: Expression)
304304
*/
305305
case object NamePlaceholder extends LeafExpression with Unevaluable {
306306
override lazy val resolved: Boolean = false
307-
override def foldable: Boolean = false
308307
override def nullable: Boolean = false
309308
override def dataType: DataType = StringType
310309
override def prettyName: String = "NamePlaceholder"

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ case class AssertTrue(child: Expression) extends UnaryExpression with ImplicitCa
113113
since = "1.6.0")
114114
case class CurrentDatabase() extends LeafExpression with Unevaluable {
115115
override def dataType: DataType = StringType
116-
override def foldable: Boolean = true
117116
override def nullable: Boolean = false
118117
override def prettyName: String = "current_database"
119118
}
@@ -131,7 +130,6 @@ case class CurrentDatabase() extends LeafExpression with Unevaluable {
131130
since = "3.1.0")
132131
case class CurrentCatalog() extends LeafExpression with Unevaluable {
133132
override def dataType: DataType = StringType
134-
override def foldable: Boolean = true
135133
override def nullable: Boolean = false
136134
override def prettyName: String = "current_catalog"
137135
}

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,6 @@ case class InSubquery(values: Seq[Expression], query: ListQuery)
337337

338338
override def children: Seq[Expression] = values :+ query
339339
override def nullable: Boolean = children.exists(_.nullable)
340-
override def foldable: Boolean = children.forall(_.foldable)
341340
override def toString: String = s"$value IN ($query)"
342341
override def sql: String = s"(${value.sql} IN (${query.sql}))"
343342
}

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ case class WindowSpecDefinition(
5050
frameSpecification.isInstanceOf[SpecifiedWindowFrame]
5151

5252
override def nullable: Boolean = true
53-
override def foldable: Boolean = false
5453
override def dataType: DataType = throw new UnsupportedOperationException("dataType")
5554

5655
override def checkInputDataTypes(): TypeCheckResult = {
@@ -144,7 +143,6 @@ case object RangeFrame extends FrameType {
144143
sealed trait SpecialFrameBoundary extends Expression with Unevaluable {
145144
override def children: Seq[Expression] = Nil
146145
override def dataType: DataType = NullType
147-
override def foldable: Boolean = false
148146
override def nullable: Boolean = false
149147
}
150148

@@ -168,7 +166,6 @@ case object CurrentRow extends SpecialFrameBoundary {
168166
sealed trait WindowFrame extends Expression with Unevaluable {
169167
override def children: Seq[Expression] = Nil
170168
override def dataType: DataType = throw new UnsupportedOperationException("dataType")
171-
override def foldable: Boolean = false
172169
override def nullable: Boolean = false
173170
}
174171

@@ -275,7 +272,6 @@ case class UnresolvedWindowExpression(
275272
windowSpec: WindowSpecReference) extends UnaryExpression with Unevaluable {
276273

277274
override def dataType: DataType = throw new UnresolvedException(this, "dataType")
278-
override def foldable: Boolean = throw new UnresolvedException(this, "foldable")
279275
override def nullable: Boolean = throw new UnresolvedException(this, "nullable")
280276
override lazy val resolved = false
281277
}
@@ -287,7 +283,6 @@ case class WindowExpression(
287283
override def children: Seq[Expression] = windowFunction :: windowSpec :: Nil
288284

289285
override def dataType: DataType = windowFunction.dataType
290-
override def foldable: Boolean = windowFunction.foldable
291286
override def nullable: Boolean = windowFunction.nullable
292287

293288
override def toString: String = s"$windowFunction $windowSpec"
@@ -370,8 +365,11 @@ abstract class OffsetWindowFunction
370365
* OffsetWindowFunction is executed, the input expression and the default expression. Even when
371366
* both the input and the default expression are foldable, the result is still not foldable due to
372367
* the frame.
368+
*
369+
* Note, the value of foldable is set to false in the trait Unevaluable
370+
*
371+
* override def foldable: Boolean = false
373372
*/
374-
override def foldable: Boolean = false
375373

376374
override def nullable: Boolean = default == null || default.nullable || input.nullable
377375

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,6 @@ case class MergeIntoTable(
348348

349349
sealed abstract class MergeAction extends Expression with Unevaluable {
350350
def condition: Option[Expression]
351-
override def foldable: Boolean = false
352351
override def nullable: Boolean = false
353352
override def dataType: DataType = throw new UnresolvedException(this, "nullable")
354353
override def children: Seq[Expression] = condition.toSeq
@@ -369,7 +368,6 @@ case class InsertAction(
369368
}
370369

371370
case class Assignment(key: Expression, value: Expression) extends Expression with Unevaluable {
372-
override def foldable: Boolean = false
373371
override def nullable: Boolean = false
374372
override def dataType: DataType = throw new UnresolvedException(this, "nullable")
375373
override def children: Seq[Expression] = key :: value :: Nil

0 commit comments

Comments
 (0)