LuciferYang commented on code in PR #59573:
URL: https://github.com/apache/spark/pull/59573#discussion_r4321960720
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/basicPhysicalOperators.scala:
##########
@@ -215,42 +323,44 @@ trait GeneratePredicateHelper extends PredicateHelper {
if !generatedIsNotNullChecks(idx) && !n.isInstanceOf[Attribute] &&
Review Comment:
This nested branch (from SPARK-51356) generates an `IsNotNull(expr)` on a
non-attribute `expr` just before the first conjunct that contains `expr`, even
when the check is written after that conjunct. Before this PR, a row with an
earlier NULL conjunct never got here. With the flag it does, and
`precedingNotNullChecks` only handles checks written before the conjunct.
On `withNullConjunctTable(Int.MaxValue)` with ANSI on and Parquet filter
pushdown off, `WHERE NOT (a > 0 AND b > 0) AND coalesce(b, cast(s AS INT)) < 0
AND cast(s AS INT) IS NOT NULL` returns no rows with codegen off and with the
legacy config, but raises `CAST_INVALID_INPUT` with codegen on and the legacy
config off, although the second conjunct is FALSE.
I tried skipping this branch, here and in the CSE branch, when
`afterNullable` is true (`nullableSeen` before `nullableSeen ||=
bound.nullable`, so the first conjunct that can be NULL keeps its check). The
checks written before the conjunct are generated by then, and the direct or
synthetic `IsNotNull(r)` still covers the binding. These queries then match
codegen off, and the SPARK-60315 and SPARK-51356 tests pass. Could you make
that change, with a test for this shape?
--
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]
