rich7420 commented on code in PR #6847:
URL: https://github.com/apache/datafusion-comet/pull/6847#discussion_r4301795451
##########
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:
Addressed at `aaf808d29`: FLOAT/DOUBLE comparisons and IN predicates are no
longer pushed, including standalone predicates and leaves under AND/OR/NOT.
Floating-point null checks and independent safe conjuncts remain eligible.
Added the mixed signed-zero regression; both the serializer and runtime
controls fail before the fix. At this head, 29 serializer tests, six targeted
native tests (including 44 floating-point queries) and [upstream
CI](https://github.com/apache/datafusion-comet/actions/runs/38160026569) passed.
Could you apply `run-iceberg-tests` for broader coverage? I tried the fork
CI path, but GitHub rejects dispatch with “Actions has been disabled for this
repository.”
--
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]
