adriangb commented on PR #17452:
URL: https://github.com/apache/datafusion/pull/17452#issuecomment-3271732259

   > @adriangb What are your thoughts on the benchmark run? I see some 
regressions 🤔
   
   I think it's noise. For example, let's look at:
   
   ```
   Benchmark clickbench_partitioned.json
   QQuery 12    │  1326.44 ms │                               1485.86 ms │  
1.12x slower
   ```
   
   
https://github.com/apache/datafusion/blob/da1e7ae2e59291499100753acd749d3a23cc88b5/benchmarks/queries/clickbench/queries/q12.sql#L4
   
   Which does not obviously have a join. And I can confirm with the query plan 
that there are no joins:
   
   ```
   > explain SELECT "SearchPhrase", COUNT(*) AS c FROM hits WHERE 
"SearchPhrase" <> '' GROUP BY "SearchPhrase" ORDER BY c DESC LIMIT 10;
   +---------------+-------------------------------+
   | plan_type     | plan                          |
   +---------------+-------------------------------+
   | physical_plan | ┌───────────────────────────┐ |
   |               | │  SortPreservingMergeExec  │ |
   |               | │    --------------------   │ |
   |               | │      c DESClimit: 10      │ |
   |               | └─────────────┬─────────────┘ |
   |               | ┌─────────────┴─────────────┐ |
   |               | │       SortExec(TopK)      │ |
   |               | │    --------------------   │ |
   |               | │          c@1 DESC         │ |
   |               | │                           │ |
   |               | │         limit: 10         │ |
   |               | └─────────────┬─────────────┘ |
   |               | ┌─────────────┴─────────────┐ |
   |               | │       ProjectionExec      │ |
   |               | │    --------------------   │ |
   |               | │       SearchPhrase:       │ |
   |               | │        SearchPhrase       │ |
   |               | │                           │ |
   |               | │     c: count(Int64(1))    │ |
   |               | └─────────────┬─────────────┘ |
   |               | ┌─────────────┴─────────────┐ |
   |               | │       AggregateExec       │ |
   |               | │    --------------------   │ |
   |               | │       aggr: count(1)      │ |
   |               | │                           │ |
   |               | │         group_by:         │ |
   |               | │        SearchPhrase       │ |
   |               | │                           │ |
   |               | │           mode:           │ |
   |               | │      FinalPartitioned     │ |
   |               | └─────────────┬─────────────┘ |
   |               | ┌─────────────┴─────────────┐ |
   |               | │    CoalesceBatchesExec    │ |
   |               | │    --------------------   │ |
   |               | │     target_batch_size:    │ |
   |               | │            8192           │ |
   |               | └─────────────┬─────────────┘ |
   |               | ┌─────────────┴─────────────┐ |
   |               | │      RepartitionExec      │ |
   |               | │    --------------------   │ |
   |               | │ partition_count(in->out): │ |
   |               | │          12 -> 12         │ |
   |               | │                           │ |
   |               | │    partitioning_scheme:   │ |
   |               | │ Hash([SearchPhrase@0], 12)│ |
   |               | └─────────────┬─────────────┘ |
   |               | ┌─────────────┴─────────────┐ |
   |               | │       AggregateExec       │ |
   |               | │    --------------------   │ |
   |               | │       aggr: count(1)      │ |
   |               | │                           │ |
   |               | │         group_by:         │ |
   |               | │        SearchPhrase       │ |
   |               | │                           │ |
   |               | │       mode: Partial       │ |
   |               | └─────────────┬─────────────┘ |
   |               | ┌─────────────┴─────────────┐ |
   |               | │    CoalesceBatchesExec    │ |
   |               | │    --------------------   │ |
   |               | │     target_batch_size:    │ |
   |               | │            8192           │ |
   |               | └─────────────┬─────────────┘ |
   |               | ┌─────────────┴─────────────┐ |
   |               | │         FilterExec        │ |
   |               | │    --------------------   │ |
   |               | │         predicate:        │ |
   |               | │      SearchPhrase !=      │ |
   |               | └─────────────┬─────────────┘ |
   |               | ┌─────────────┴─────────────┐ |
   |               | │       DataSourceExec      │ |
   |               | │    --------------------   │ |
   |               | │         files: 111        │ |
   |               | │      format: parquet      │ |
   |               | │                           │ |
   |               | │         predicate:        │ |
   |               | │      SearchPhrase !=      │ |
   |               | └───────────────────────────┘ |
   |               |                               |
   +---------------+-------------------------------+
   ```
   
   So a regression in this query could not possibly be caused by the changes in 
this PR which are narrowly scoped to HashJoinExec.


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