cloud-fan commented on code in PR #51146:
URL: https://github.com/apache/spark/pull/51146#discussion_r2161159546
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/V2ExpressionBuilder.scala:
##########
@@ -78,6 +78,7 @@ class V2ExpressionBuilder(e: Expression, isPredicate: Boolean
= false) extends L
expr: Expression, isPredicate: Boolean = false): Option[V2Expression] =
expr match {
case Literal(true, BooleanType) => Some(new AlwaysTrue())
case Literal(false, BooleanType) => Some(new AlwaysFalse())
+ case Cast(Literal(null, NullType), BooleanType, _, _) if isPredicate =>
Some(new AlwaysNull())
Review Comment:
I think the v2 `Predicate` is a big mistake... We never know if an
expression is a predicate or not statically. In fact, we only require
boolean-type expressions in catalyst to be used as predicates.
In this case, a null literal can be a predicate, a CAST to boolean is a
predicate, but they do not extend the v2 `Predicate` class and we can't make
the code compile.
It's too late to remove v2 `Predicate` as the ship is already sailed, but we
can silently kill it by adding a new `BooleanExpression` v2 expression that
extends `Predicate` and simply wraps any v2 expressions that returns boolean
type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
