LinSimon-901101 commented on code in PR #7155:
URL: https://github.com/apache/datafusion-comet/pull/7155#discussion_r4373516240
##########
spark/src/main/scala/org/apache/comet/serde/predicates.scala:
##########
@@ -491,3 +495,51 @@ object ComparisonUtils {
}
}
}
+
+object CometAssertNotNull extends CometExpressionSerde[AssertNotNull] {
+ // Enabling this previously unsupported expression must not make an
enclosing eager kernel
+ // check rows that Spark skips (for example, false AND assertnotnull(NULL)).
Admit the
+ // structural and lazy conditional contexts used by V2 writes; keep other
compositions in
+ // Spark until their evaluation order has been verified. Check every parent
during serde so
+ // a safe wrapper cannot hide an unsafe descendant.
+ private[serde] def hasUnsupportedParent(expr: Expression, proto: Expr):
Boolean = expr match {
+ case _: Alias | _: AssertNotNull | _: CreateNamedStruct | _:
GetStructField | _: If |
+ _: CaseWhen =>
+ false
+ case _ =>
+ expr.containsPattern(NULL_CHECK) && containsNativeAssertion(proto)
Review Comment:
Sure, I fixed it in `b77bf16`, included in the current head `6e850d7`.
In that head, I use `hasAssertionCandidate` at `predicates.scala:500–516` to
check for an actual Catalyst `AssertNotNull` before the proto walk, so ordinary
`IsNull` and `IsNotNull` expressions skip the proto walk.
--
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]
