Skip to content

Conversation

adriangb
Copy link
Contributor

Replaces a piece of #16445 to break up into smaller PRs

@github-actions github-actions bot added physical-plan Changes to the physical-plan crate core Core DataFusion crate labels Jul 28, 2025
@alamb
Copy link
Contributor

alamb commented Jul 28, 2025

🤖 ./gh_compare_branch.sh Benchmark Script Running
Linux aal-dev 6.11.0-1016-gcp #16~24.04.1-Ubuntu SMP Wed May 28 02:40:52 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing hash-join-allow-pushdown (2f76a91) to c6d5520 diff using: tpch_mem clickbench_partitioned clickbench_extended
Results will be posted here when complete

@alamb
Copy link
Contributor

alamb commented Jul 28, 2025

I will try and review this PR tomorrow

@adriangb
Copy link
Contributor Author

Results will be posted here when complete

FWIW my expectation is that this won't make much of a difference unless there is a chain like TopK -> HashJoin -> Scan and filter pushdown into the scan is enabled

@alamb
Copy link
Contributor

alamb commented Jul 28, 2025

🤖: Benchmark completed

Details

Comparing HEAD and hash-join-allow-pushdown
--------------------
Benchmark clickbench_extended.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Query        ┃        HEAD ┃ hash-join-allow-pushdown ┃    Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ QQuery 0     │  1934.91 ms │               1967.56 ms │ no change │
│ QQuery 1     │   680.09 ms │                686.86 ms │ no change │
│ QQuery 2     │  1343.67 ms │               1367.13 ms │ no change │
│ QQuery 3     │   671.54 ms │                670.78 ms │ no change │
│ QQuery 4     │  1343.96 ms │               1369.05 ms │ no change │
│ QQuery 5     │ 14659.89 ms │              14644.36 ms │ no change │
│ QQuery 6     │  2072.69 ms │               2065.36 ms │ no change │
│ QQuery 7     │  1921.02 ms │               1962.28 ms │ no change │
└──────────────┴─────────────┴──────────────────────────┴───────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary                       ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)                       │ 24627.76ms │
│ Total Time (hash-join-allow-pushdown)   │ 24733.38ms │
│ Average Time (HEAD)                     │  3078.47ms │
│ Average Time (hash-join-allow-pushdown) │  3091.67ms │
│ Queries Faster                          │          0 │
│ Queries Slower                          │          0 │
│ Queries with No Change                  │          8 │
│ Queries with Failure                    │          0 │
└─────────────────────────────────────────┴────────────┘
--------------------
Benchmark clickbench_partitioned.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━┓
┃ Query        ┃        HEAD ┃ hash-join-allow-pushdown ┃       Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━┩
│ QQuery 0     │     2.33 ms │                  2.75 ms │ 1.18x slower │
│ QQuery 1     │    33.29 ms │                 34.53 ms │    no change │
│ QQuery 2     │    80.43 ms │                 80.99 ms │    no change │
│ QQuery 3     │    98.30 ms │                 96.81 ms │    no change │
│ QQuery 4     │   598.15 ms │                609.25 ms │    no change │
│ QQuery 5     │   848.89 ms │                853.05 ms │    no change │
│ QQuery 6     │     2.24 ms │                  2.26 ms │    no change │
│ QQuery 7     │    37.45 ms │                 38.15 ms │    no change │
│ QQuery 8     │   848.13 ms │                856.08 ms │    no change │
│ QQuery 9     │  1180.14 ms │               1200.91 ms │    no change │
│ QQuery 10    │   257.14 ms │                259.27 ms │    no change │
│ QQuery 11    │   292.14 ms │                286.57 ms │    no change │
│ QQuery 12    │   877.62 ms │                882.69 ms │    no change │
│ QQuery 13    │  1234.54 ms │               1242.03 ms │    no change │
│ QQuery 14    │   838.08 ms │                828.27 ms │    no change │
│ QQuery 15    │   774.78 ms │                792.96 ms │    no change │
│ QQuery 16    │  1668.70 ms │               1625.33 ms │    no change │
│ QQuery 17    │  1625.72 ms │               1618.61 ms │    no change │
│ QQuery 18    │  2860.56 ms │               2894.43 ms │    no change │
│ QQuery 19    │    88.99 ms │                 87.42 ms │    no change │
│ QQuery 20    │  1195.22 ms │               1179.02 ms │    no change │
│ QQuery 21    │  1334.91 ms │               1336.30 ms │    no change │
│ QQuery 22    │  2194.96 ms │               2217.46 ms │    no change │
│ QQuery 23    │  7536.86 ms │               7550.03 ms │    no change │
│ QQuery 24    │   443.15 ms │                446.15 ms │    no change │
│ QQuery 25    │   315.38 ms │                310.61 ms │    no change │
│ QQuery 26    │   435.79 ms │                441.80 ms │    no change │
│ QQuery 27    │  1562.84 ms │               1568.25 ms │    no change │
│ QQuery 28    │ 12113.23 ms │              12088.54 ms │    no change │
│ QQuery 29    │   530.52 ms │                526.00 ms │    no change │
│ QQuery 30    │   801.92 ms │                803.02 ms │    no change │
│ QQuery 31    │   791.77 ms │                798.40 ms │    no change │
│ QQuery 32    │  2419.43 ms │               2478.76 ms │    no change │
│ QQuery 33    │  3197.37 ms │               3205.40 ms │    no change │
│ QQuery 34    │  3223.48 ms │               3259.30 ms │    no change │
│ QQuery 35    │  1252.88 ms │               1242.37 ms │    no change │
│ QQuery 36    │   121.83 ms │                122.84 ms │    no change │
│ QQuery 37    │    52.28 ms │                 54.31 ms │    no change │
│ QQuery 38    │   121.19 ms │                122.45 ms │    no change │
│ QQuery 39    │   199.03 ms │                197.62 ms │    no change │
│ QQuery 40    │    42.09 ms │                 42.93 ms │    no change │
│ QQuery 41    │    38.03 ms │                 39.56 ms │    no change │
│ QQuery 42    │    33.06 ms │                 32.59 ms │    no change │
└──────────────┴─────────────┴──────────────────────────┴──────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary                       ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)                       │ 54204.85ms │
│ Total Time (hash-join-allow-pushdown)   │ 54356.11ms │
│ Average Time (HEAD)                     │  1260.58ms │
│ Average Time (hash-join-allow-pushdown) │  1264.10ms │
│ Queries Faster                          │          0 │
│ Queries Slower                          │          1 │
│ Queries with No Change                  │         42 │
│ Queries with Failure                    │          0 │
└─────────────────────────────────────────┴────────────┘
--------------------
Benchmark tpch_mem_sf1.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━┓
┃ Query        ┃      HEAD ┃ hash-join-allow-pushdown ┃       Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━┩
│ QQuery 1     │  95.09 ms │                101.86 ms │ 1.07x slower │
│ QQuery 2     │  20.99 ms │                 20.09 ms │    no change │
│ QQuery 3     │  33.04 ms │                 32.87 ms │    no change │
│ QQuery 4     │  18.26 ms │                 18.58 ms │    no change │
│ QQuery 5     │  49.55 ms │                 49.54 ms │    no change │
│ QQuery 6     │  11.91 ms │                 12.02 ms │    no change │
│ QQuery 7     │  86.50 ms │                 88.81 ms │    no change │
│ QQuery 8     │  24.43 ms │                 25.01 ms │    no change │
│ QQuery 9     │  53.68 ms │                 53.40 ms │    no change │
│ QQuery 10    │  42.33 ms │                 42.27 ms │    no change │
│ QQuery 11    │  11.51 ms │                 11.07 ms │    no change │
│ QQuery 12    │  34.99 ms │                 35.18 ms │    no change │
│ QQuery 13    │  25.84 ms │                 26.41 ms │    no change │
│ QQuery 14    │   9.70 ms │                  9.70 ms │    no change │
│ QQuery 15    │  18.93 ms │                 19.36 ms │    no change │
│ QQuery 16    │  18.21 ms │                 18.01 ms │    no change │
│ QQuery 17    │  96.23 ms │                 97.18 ms │    no change │
│ QQuery 18    │ 198.58 ms │                191.80 ms │    no change │
│ QQuery 19    │  25.71 ms │                 24.59 ms │    no change │
│ QQuery 20    │  30.84 ms │                 31.70 ms │    no change │
│ QQuery 21    │ 145.91 ms │                142.65 ms │    no change │
│ QQuery 22    │  15.11 ms │                 14.55 ms │    no change │
└──────────────┴───────────┴──────────────────────────┴──────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Benchmark Summary                       ┃           ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ Total Time (HEAD)                       │ 1067.31ms │
│ Total Time (hash-join-allow-pushdown)   │ 1066.65ms │
│ Average Time (HEAD)                     │   48.51ms │
│ Average Time (hash-join-allow-pushdown) │   48.48ms │
│ Queries Faster                          │         0 │
│ Queries Slower                          │         1 │
│ Queries with No Change                  │        21 │
│ Queries with Failure                    │         0 │
└─────────────────────────────────────────┴───────────┘

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @adriangb -- this PR makes sense to me

In case anyone else finds this -- it allows physical pushdown through joins of dynamic filters but as @adriangb mentions, there are not that many dynamic filters yet

parent_filters: Vec<Arc<dyn PhysicalExpr>>,
_config: &ConfigOptions,
) -> Result<FilterDescription> {
if self.join_type != JoinType::Inner {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend we file a ticket and then add a reference to that ticket in the code a ticket reference so someone in the future who stumbles on this code can locate the relevant context

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adriangb adriangb force-pushed the hash-join-allow-pushdown branch from e767f04 to 4876099 Compare July 30, 2025 18:29
@adriangb adriangb merged commit b10f453 into apache:main Jul 30, 2025
27 checks passed
Copy link
Member

@xudong963 xudong963 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for late, looking forward to using it in DF50

Standing-Man pushed a commit to Standing-Man/datafusion that referenced this pull request Aug 4, 2025
LiaCastaneda pushed a commit to DataDog/datafusion that referenced this pull request Sep 2, 2025
LiaCastaneda added a commit to DataDog/datafusion that referenced this pull request Sep 9, 2025
* Enable physical filter pushdown for hash joins (apache#16954)

(cherry picked from commit b10f453)

* Add ExecutionPlan::reset_state (apache#17028)

* Add ExecutionPlan::reset_state

Co-authored-by: Robert Ream <[email protected]>

* Update datafusion/sqllogictest/test_files/cte.slt

* Add reference

* fmt

* add to upgrade guide

* add explain plan, implement in more plans

* fmt

* only explain

---------

Co-authored-by: Robert Ream <[email protected]>

* Add dynamic filter (bounds) pushdown to HashJoinExec (apache#16445)

(cherry picked from commit ff77b70)

* Push dynamic pushdown through CooperativeExec and ProjectionExec (apache#17238)

(cherry picked from commit 4bc0696)

* Fix dynamic filter pushdown in HashJoinExec (apache#17201)

(cherry picked from commit 1d4d74b)

* Fix HashJoinExec sideways information passing for partitioned queries (apache#17197)

(cherry picked from commit 64bc58d)

* disallow pushdown of volatile functions (apache#16861)

* dissallow pushdown of volatile PhysicalExprs

* fix

* add FilteredVec helper to handle filter / remap pattern (#34)

* checkpoint: Address PR feedback in https://github.com/apach...

* add FilteredVec to consolidate handling of filter / remap pattern

* lint

* Add slt test for pushing volatile predicates down (#35)

---------

Co-authored-by: Andrew Lamb <[email protected]>
(cherry picked from commit 94e8548)

* fix bounds accumulator reset in HashJoinExec dynamic filter pushdown (apache#17371)

---------

Co-authored-by: Adrian Garcia Badaracco <[email protected]>
Co-authored-by: Robert Ream <[email protected]>
Co-authored-by: Jack Kleeman <[email protected]>
Co-authored-by: Andrew Lamb <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate physical-plan Changes to the physical-plan crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants