dwsmith1983 opened a new pull request, #25657:
URL: https://github.com/apache/datafusion/pull/25657

   ## Which issue does this PR close?
   
   - Related to #17098.
   
   ## Rationale for this change
   
   Once the TopK heap is full, most input batches are rejected outright by the 
threshold filter. `insert_batch` still evaluated every sort expression over the 
whole batch before checking the filter, then evaluated the same expressions 
again inside the filter, and cloned the batch even when nothing from it was 
kept. With an expression sort key that doubles the per-batch cost for no output.
   
   ## What changes are included in this PR?
   
   - The threshold filter runs before the sort keys are evaluated; fully 
rejected batches skip sort-key evaluation.
   - The batch moves into the heap entry instead of being cloned. The last 
row's common prefix is encoded before the move so early completion still 
compares against the same boundary at the same point.
   - The per-batch timer clones one metric instead of the whole baseline set, 
here and in `PartitionedTopK`.
   
   `SELECT l_orderkey, l_extendedprice FROM lineitem ORDER BY l_extendedprice + 
0 DESC LIMIT 1` on TPC-H SF1, TopK `elapsed_compute`, median of five: 152 ms 
before, 94 ms after. `sort-tpch --limit 10` shows no change.
   
   One nuance: a sort-key expression that errors only on rows the threshold 
already rejects no longer raises, because those batches skip sort-key 
evaluation. The filter evaluates the same expression over the batch first, so 
this can only differ for a secondary key under short-circuit evaluation.
   
   ## What is the testing strategy for this PR?
   
   New unit tests in `datafusion/physical-plan/src/topk`: 
`test_batch_rejected_by_threshold_leaves_heap_untouched` and 
`test_expression_sort_key_matches_column_sort_key`. Existing TopK tests and the 
`topk`, `window` and `limit` sqllogictest files pass unchanged.
   
   ## Are there any user-facing changes?
   
   No.
   


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