sunchao commented on code in PR #6847:
URL: https://github.com/apache/datafusion-comet/pull/6847#discussion_r4345061265
##########
spark/src/main/scala/org/apache/comet/serde/operator/CometIcebergNativeScan.scala:
##########
@@ -1009,6 +1082,18 @@ object CometIcebergNativeScan extends
CometOperatorSerde[CometBatchScanExec] wit
// schema so a filter column projected out of the scan output is still
recognized.
val pageIndexUnsupportedColumns =
IcebergReflection.pageIndexUnsupportedColumns(metadata.tableSchema)
+ // Columns whose type changed over the table's history; residual
predicates comparing them
+ // with a literal are dropped, except equality. The scan schema also
covers a column that
+ // VERSION AS OF reads after it was dropped from the table.
+ val promotedColumns = Seq(metadata.tableSchema, metadata.scanSchema)
+ .flatMap(IcebergReflection.promotedColumns(metadata.table, _))
+ .toSet
+ // add_files, migrate and snapshot set a name mapping and import files
without checking their
+ // types against the schema. Such files commonly hold narrower physical
types, such as Hive
+ // TINYINT and SMALLINT stored as INT32 with INT_8 or INT_16 annotations,
which iceberg-rust
+ // compares incorrectly. So partial pushdown is not extended to these
tables, and they push a
+ // residual only when it converts whole.
+ val partialPushdown = metadata.nameMapping.isEmpty
Review Comment:
[P1] [P1] Preserve rows from unrecorded promotions when enabling partial
pushdown. A table whose `id` was promoted from `INT` to `BIGINT` before Iceberg
0.12 can have physical `INT` files, no name mapping, and only schema ID `0` in
its reconstructed history. `promotedColumns` consequently returns empty, and
this gate enables partial pushdown. For rows `(1, array(1)), (2, array(2)), (3,
array(3))`, `WHERE l IS NOT NULL AND id < 3000000000` should return all three
rows. The base drops the mixed residual, but this head retains `NotNull(id) AND
LessThan(id, 3000000000)`, and the pinned reader drops every row. This extends
#7138 to queries that previously returned correct results. Please retain a
conservative partial-pushdown guard for potentially promoted columns unless
physical types are verified, or fix the native comparison before enabling this
path.
Evidence: Reproduced in `/tmp/review6847-622d-1c0zesk4`.
`LegacyPromotionProbe.scala` creates an unpartitioned table with an INT Parquet
file, promotes its logical column to BIGINT, and writes the supported v1
schema-only metadata representation emitted by Iceberg 0.11.1. Upstream
old/current TableMetadataParser sources confirm this representation and its
reconstruction as a single schema. `legacy-spark.log` records schema IDs `[0]`,
`promoted=Set()`, no name mapping, Spark results `[1,2,3]`, base serialization
`None`, and exact-head serialization containing the retained range.
`legacy-native.log` records the same file returning three rows without pushdown
and zero with the emitted `NotNull(id) AND id < 3000000000` predicate. The
reader uses iceberg-rust `1f3bc34058e3f3e45336bc5428f37b92649a608b`, Arrow
59.3.0, `rewrite_not().bind(...)`, and row selection enabled.
--
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]
