rich7420 commented on PR #6847: URL: https://github.com/apache/datafusion-comet/pull/6847#issuecomment-6333809953
Thanks @mbutrovich. I split the fixes for #7136, #7137 and the promoted-column part of #7138 into #7156, and rebased this PR on top of it. On top of #7156, this PR only adds partial pushdown, so the benchmark below compares it with #7156, not with `main`. On "native Iceberg pruning": `ArrowReader` can use the pushed conjuncts to prune row groups using statistics, select rows using the page index and filter rows with the `RowFilter` during decode. I updated the description to say so. I also simplified the change. The serde converts the residual once, and each kept conjunct goes through the same rules as a residual that converts in full. The earlier version also restricted every `BIGINT` and `DOUBLE` column to `=` when only part of the residual converted, which blocked pruning on `BIGINT` keys. Benchmark: I added partial filter cases to `CometIcebergReadBenchmark` (16M rows, 8 files, each spanning the whole key range, so Iceberg's file pruning skips nothing). Each query is `l IS NOT NULL AND <range>`, and only the range can be pushed. Each timing cell shows the best Comet time from each of two runs (run 1 / run 2): | Pushed range | #7156 | This PR | Bytes scanned | |---|---|---|---| | 1% of a clustered `INT` column | 1303 / 1276 ms | 67 / 63 ms | 663 MiB → 14 MiB | | 1% of a clustered `BIGINT` column | 1308 / 1284 ms | 64 / 67 ms | 633 MiB → 13 MiB | | 1% of a scattered `INT` column | 1265 / 1245 ms | 1232 / 1208 ms | 671 MiB → 671 MiB | | 90% of a scattered `INT` column | 1358 / 1367 ms | 1466 / 1508 ms | 671 MiB → 671 MiB | The 90% scattered case was the least favorable case measured: statistics skip nothing and the `RowFilter` removes few rows, and it was 8–10% slower in these runs. Within a single build, the timings with the range pushed and with it written as `scattered + 0` to prevent pushdown differed by 0–5%, so part of that gap is run-to-run variation. Separately, while checking promoted columns I found that iceberg-rust's page-index evaluator can wrongly skip pages whose bounds have a narrower type than the table column, such as `INT32` pages on a `BIGINT` column, which drops matching rows. I opened apache/iceberg-rust#3507 to fix it. -- 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]
