sunchao commented on code in PR #6847:
URL: https://github.com/apache/datafusion-comet/pull/6847#discussion_r4315888505
##########
spark/src/main/scala/org/apache/comet/serde/operator/CometIcebergNativeScan.scala:
##########
@@ -733,36 +740,42 @@ object CometIcebergNativeScan extends
CometOperatorSerde[CometBatchScanExec] wit
val left = icebergExprToProto(
IcebergReflection.getMethod(exprClass, "left").invoke(icebergExpr),
output,
- pageIndexUnsupportedColumns)
+ pageIndexUnsupportedColumns,
+ allowPartial)
val right = icebergExprToProto(
IcebergReflection.getMethod(exprClass, "right").invoke(icebergExpr),
output,
- pageIndexUnsupportedColumns)
+ pageIndexUnsupportedColumns,
+ allowPartial)
(left, right) match {
- // Push the residual only if it converts whole. Dropping a conjunct is
safe in positive
- // position but strengthens the predicate under a NOT (De Morgan),
which would wrongly
- // prune, and tracking polarity across arbitrary nesting is error
prone. So an
- // unconvertible conjunct elides the whole residual; the post-scan
CometFilter is exact.
case (Some(l), Some(r)) => Some(logicalPredicate(isAnd = true, l, r))
+ // Dropping a positive conjunct weakens the predicate; the post-scan
filter stays exact.
+ // Inside NOT, every descendant must convert exactly, including nested
AND/OR nodes.
+ case (l, r) if allowPartial => l.orElse(r)
Review Comment:
[P1] [P1] Preserve NULL matches in retained negated null-safe comparisons.
With native Iceberg scanning enabled, create an unpartitioned `(id INT, l
ARRAY<INT>)` table containing `(NULL, array(1))`, `(1, array(1))`, and `(2,
array(1))`, then query `WHERE l IS NOT NULL AND NOT (id <=> 1)`. Spark returns
the NULL and 2 rows. The base omits this mixed residual, but this branch now
retains `NOT(Eq(id, 1))`. Native `rewrite_not()` turns it into `NotEq`, whose
Arrow comparison produces NULL for a null input and filters that row out. Only
ID 2 survives, and the post-scan filter cannot recover the missing row. Please
decline these null-sensitive negative residuals, including those produced by
`NOT`, or preserve their null-inclusive semantics in native filtering, with a
regression for this mixed predicate.
Evidence: Fresh reproduction in `/tmp/comet6847-review-20261019-phKbuI`:
`nullsafe-spark.log` records Spark 4.1.3/Iceberg 1.11.0 returning `[NULL, 2]`,
the actual residual `(not_null(l) and not(id == 1))`, base serialization
`None`, and exact-head serialization `NOT(Eq(id, 1))`. `nullsafe-native.log`
records the same Parquet file returning `[NULL, 1, 2]` without pushdown and
`[2]` with the retained predicate, leaving two versus one rows after the
original condition. The reader uses iceberg-rust
`af1da4c5bc86178c38c1c6db0578bdd9fb7021e3`, Arrow 59.3.0,
`rewrite_not().bind(...)`, and `with_row_selection_enabled(true)`, matching the
checkout. Its `predicate_visitor.rs::not_eq` calls Arrow `neq`, which returns
null for null input.
--
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]
