Skip to content

Improve HashJoin::needUsedFlagsForPerRightTableRow, returns false for cross join#82379

Merged
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
bigo-sg:impr_hj
Jul 13, 2025
Merged

Improve HashJoin::needUsedFlagsForPerRightTableRow, returns false for cross join#82379
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
bigo-sg:impr_hj

Conversation

@lgbo-ustc
Copy link
Copy Markdown
Contributor

@lgbo-ustc lgbo-ustc commented Jun 23, 2025

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Improve HashJoin::needUsedFlagsForPerRightTableRow, returns false for cross join

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@yariks5s yariks5s added the can be tested Allows running workflows for external contributors label Jun 23, 2025
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Jun 23, 2025

Workflow [PR], commit [4bdf5f0]

Summary:

@clickhouse-gh clickhouse-gh bot added the pr-improvement Pull request with some product improvements label Jun 23, 2025
@yariks5s
Copy link
Copy Markdown
Member

It's a bit concerning. We anyway get the same result, after checking the two if-statements. It doesn't seem like it's changing something. Why do we prioritize CROSS JOIN over other JOINs?

@yariks5s yariks5s self-assigned this Jun 23, 2025
@lgbo-ustc
Copy link
Copy Markdown
Contributor Author

lgbo-ustc commented Jun 24, 2025

It's a bit concerning. We anyway get the same result, after checking the two if-statements. It doesn't seem like it's changing something. Why do we prioritize CROSS JOIN over other JOINs?

In our case, we build a broadcast join for cross join which will reuse the same join data. needUsedFlagsForPerRightTable returns true and causes an exception. Currently, since CH doesn't support reusing join data for cross join, the needUsedFlagsForPerRightTable logic functions correctly. However, we believe prioritizing CROSS JOIN over other join types would be OK, and has more complete logic coverage.

@baibaichen
Copy link
Copy Markdown
Contributor

@yariks5s Could we merge this PR?

baibaichen pushed a commit to Kyligence/ClickHouse that referenced this pull request Jun 27, 2025
baibaichen pushed a commit to Kyligence/gluten that referenced this pull request Jun 27, 2025
baibaichen pushed a commit to Kyligence/ClickHouse that referenced this pull request Jun 27, 2025
baibaichen pushed a commit to Kyligence/gluten that referenced this pull request Jun 27, 2025
baibaichen pushed a commit to Kyligence/ClickHouse that referenced this pull request Jun 28, 2025
baibaichen pushed a commit to Kyligence/gluten that referenced this pull request Jun 28, 2025
baibaichen pushed a commit to Kyligence/ClickHouse that referenced this pull request Jun 29, 2025
baibaichen pushed a commit to Kyligence/gluten that referenced this pull request Jun 29, 2025
baibaichen pushed a commit to apache/gluten that referenced this pull request Jun 29, 2025
* [GLUTEN-1632][CH]Daily Update Clickhouse Version (20250629)

* Fix Build due to ClickHouse/ClickHouse#80931

* Fix Build due to ClickHouse/ClickHouse#81976

* Fix Build due to  ClickHouse/ClickHouse#82508

* Try to Fix issue caused by ClickHouse/ClickHouse#81754

see ClickHouse/ClickHouse#82379

* Fix UT due to ClickHouse/ClickHouse#82358

---------

Co-authored-by: kyligence-git <[email protected]>
Co-authored-by: Chang chen <[email protected]>
kyligence-git pushed a commit to Kyligence/ClickHouse that referenced this pull request Jun 29, 2025
baibaichen pushed a commit to Kyligence/ClickHouse that referenced this pull request Jul 3, 2025
baibaichen pushed a commit to Kyligence/ClickHouse that referenced this pull request Jul 4, 2025
baibaichen pushed a commit to Kyligence/ClickHouse that referenced this pull request Jul 5, 2025
kyligence-git pushed a commit to Kyligence/ClickHouse that referenced this pull request Jul 6, 2025
baibaichen pushed a commit to Kyligence/ClickHouse that referenced this pull request Jul 7, 2025
kyligence-git pushed a commit to Kyligence/ClickHouse that referenced this pull request Jul 7, 2025
kyligence-git pushed a commit to Kyligence/ClickHouse that referenced this pull request Jul 8, 2025
kyligence-git pushed a commit to Kyligence/ClickHouse that referenced this pull request Jul 9, 2025
kyligence-git pushed a commit to Kyligence/ClickHouse that referenced this pull request Jul 10, 2025
kyligence-git pushed a commit to Kyligence/ClickHouse that referenced this pull request Jul 11, 2025
@alexey-milovidov alexey-milovidov added this pull request to the merge queue Jul 13, 2025
Merged via the queue into ClickHouse:master with commit 6532c8d Jul 13, 2025
121 checks passed
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

can be tested Allows running workflows for external contributors pr-improvement Pull request with some product improvements pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants