rich7420 commented on code in PR #6847:
URL: https://github.com/apache/datafusion-comet/pull/6847#discussion_r4349337145
##########
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:
Thanks, you're right. Before this PR the array null check kept the whole
residual out of the scan, so these queries returned the right rows, and partial
pushdown exposed them to #7138.
Fixed in b5fc1be16. The residual is first converted with #7156's rules. Only
if that fails and the table allows partial pushdown is it converted again
keeping the supported conjuncts. That second pass treats every `BIGINT`,
`DOUBLE` and `DECIMAL` column as promoted, since the schema history cannot rule
out a promotion made before 0.12, so they keep only null checks and `=` outside
`NOT`. Residuals that convert whole are unchanged from #7156.
The new test `partial residuals do not compare columns promoted before
Iceberg 0.12` appends a file written with `INT` and `FLOAT` columns to a
`BIGINT`/`DOUBLE` table with one schema and no name mapping. It checks `l IS
NOT NULL AND` each of `id > 0`, `id < 3000000000`, `id != 3000000000`, `id IN
(1, 3000000000)` and `d != 0.1`, with `id = 2` and `d = 0.5` as controls.
Without the fix it fails on `id > 0`: Spark returns 2 rows and Comet 0, because
the page index skips the `INT32` page.
--
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]
