rich7420 commented on code in PR #7156:
URL: https://github.com/apache/datafusion-comet/pull/7156#discussion_r4403794318
##########
spark/src/test/scala/org/apache/comet/CometIcebergNativeSuite.scala:
##########
@@ -4306,6 +4306,229 @@ class CometIcebergNativeSuite
}
}
+ /**
+ * Compares `SELECT id FROM table WHERE predicate` with Spark for each
predicate, alone and
+ * after `l IS NOT NULL AND`. That array null check is a conjunct the serde
never pushes, as in
+ * the queries that found these bugs, so each predicate is also checked in a
residual left
+ * unpushed.
+ */
+ private def checkResidualPredicates(table: String, predicates: Seq[String]):
Unit =
+ for (predicate <- predicates; prefix <- Seq("", "l IS NOT NULL AND ")) {
Review Comment:
Removed here; #6847 retains the partial-conjunct tests.
##########
spark/src/main/scala/org/apache/comet/serde/operator/CometIcebergNativeScan.scala:
##########
@@ -720,6 +738,24 @@ object CometIcebergNativeScan extends
CometOperatorSerde[CometBatchScanExec] wit
case IS_NULL => Some(unaryPredicate(columnName,
IcebergPredicateOperator.IsNull))
case IS_NOT_NULL | NOT_NULL =>
Some(unaryPredicate(columnName,
IcebergPredicateOperator.NotNull))
+ // Files written before the promotion still hold the older type,
which iceberg-rust
+ // compares with a literal incorrectly (see
IcebergReflection.promotedColumns),
+ // except for equality outside NOT: pruning keeps every row
group and page for it,
+ // and a literal the older type cannot hold matches no row there
anyway.
+ case op if promotedColumns.contains(columnName) && (op != EQ ||
underNot) =>
+ None
+ // Iceberg converts Spark's null-safe `id <=> 1` to `id = 1`,
which Spark evaluates
+ // to false for a NULL id and the native reader to NULL. Both
reject the row, but
+ // for `NOT (id <=> 1)` Spark keeps it while the native reader
still drops it. A
+ // NOT_NULL conjunct of an enclosing AND makes the column
non-nullable here.
+ case EQ if underNot && attribute.nullable => None
+ // Spark sorts every NaN above all numbers. The native row
filter sorts negative NaNs
+ // below them, and iceberg-rust prunes row groups on min/max
statistics that can
+ // leave NaN out, or hold it as both when parquet-mr 1.9 or
earlier starts a row
+ // group with NaN (apache/iceberg-rust#3494).
Review Comment:
Referenced [#3502](https://github.com/apache/iceberg-rust/pull/3502) and
retained the page-index caveat.
--
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]
