andygrove opened a new pull request, #6836:
URL: https://github.com/apache/datafusion-comet/pull/6836

   ## Which issue does this PR close?
   
   Third correctness item of the in-memory cache audit, #6834.
   
   ## Rationale for this change
   
   Spark's `PartitionBatchPruningSuite` checks cache pruning through the 
`readPartitions` and `readBatches` accumulators of `InMemoryTableScanExec`, 
which Spark's scan fills under `spark.sql.inMemoryTableScanStatistics.enable`. 
`CometInMemoryTableScanExec` had none, so the Spark SQL diffs made the suite 
assert only the answer when a Comet scan was present. That verifies nothing 
about pruning: the answer is the same whether a batch was pruned or decoded and 
filtered. With the cache on by default since #5634, Spark's forty-odd pruning 
cases have been running against Comet's format without checking that any batch 
was skipped.
   
   ## What changes are included in this PR?
   
   - `CometInMemoryTableScanExec` gets the same two accumulators, counted after 
the statistics filter in `doExecuteColumnar` as Spark counts them in 
`doExecute`, and only when the config is on.
   - The 3.5.9, 4.0.4, 4.1.3 and 4.2.0 diffs restore the suite's original 
assertions with a case for the Comet scan. The 3.4.3 diff never touched the 
suite because the cache is opt-in there. Regenerated from Spark worktrees at 
each tag per the contributor guide, and each regenerated diff applies cleanly 
to a fresh checkout of its tag.
   
   Every expectation in the suite matches what Comet's statistics can prune: 
integer and string bounds, null counts, `StartsWith` on a default-collation 
string, no pruning on arrays, binary or an `InSet`, and the 
`partitionPruning=false` case.
   
   ## How are these changes tested?
   
   - New `CometInMemoryCacheSuite` test "counts the partitions and batches a 
scan reads": one of twenty batches for a narrow range, all twenty with pruning 
off or no predicate, none for an impossible predicate, with AQE on and off. On 
Spark 3.5+ the scan then sits in a table cache stage, and the instance that ran 
has to be the one the final plan holds.
   - The full `CometInMemoryCacheSuite` passes locally on Spark 4.1 (69 tests).
   - `run-spark-4.1-tests` for `PartitionBatchPruningSuite` under Comet.
   


-- 
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]

Reply via email to