Hash join: automatically choose build side by vdimir · Pull Request #68682 · ClickHouse/ClickHouse

vdimir · 2024-08-21T19:33:18Z

Changelog category (leave one):

New Feature

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

Added an option to select the side of the join that will act as the inner table in the query plan. This is controlled by query_plan_join_inner_table_selection, which can be set to auto. In this mode, ClickHouse will try to choose the table with the smallest number of rows. Resubmitted Resubmit #68682 #71577

Documentation entry for user-facing changes

Documentation is written (mandatory for new features)

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

CI Settings (Only check the boxes if you know what you are doing):

Allow: All Required Checks
Allow: Stateless tests
Allow: Stateful tests
Allow: Integration Tests
Allow: Performance tests
Allow: All Builds
Allow: batch 1, 2 for multi-batch jobs
Allow: batch 3, 4, 5, 6 for multi-batch jobs

Exclude: Style check
Exclude: Fast test
Exclude: All with ASAN
Exclude: All with TSAN, MSAN, UBSAN, Coverage
Exclude: All with aarch64, release, debug

Run only fuzzers related jobs (libFuzzer fuzzers, AST fuzzers, etc.)
Exclude: AST fuzzers

robot-clickhouse · 2024-08-21T19:34:22Z

This is an automated comment for commit b5e3df9 with description of existing statuses. It's updated for the latest CI running

❌ Click here to open a full report in a separate page

Check name	Description	Status
Stateless tests	Runs stateless functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	❌ failure

Successful checks

Check name	Description	Status
AST fuzzer	Runs randomly generated queries to catch program errors. The build type is optionally given in parenthesis. If it fails, ask a maintainer for help	✅ success
Builds	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
ClickBench	Runs [ClickBench](https://github.com/ClickHouse/ClickBench/) with instant-attach table	✅ success
Compatibility check	Checks that clickhouse binary runs on distributions with old libc versions. If it fails, ask a maintainer for help	✅ success
Docker keeper image	The check to build and optionally push the mentioned image to docker hub	✅ success
Docker server image	The check to build and optionally push the mentioned image to docker hub	✅ success
Docs check	Builds and tests the documentation	✅ success
Flaky tests	Checks if new added or modified tests are flaky by running them repeatedly, in parallel, with more randomization. Functional tests are run 100 times with address sanitizer, and additional randomization of thread scheduling. Integration tests are run up to 10 times. If at least once a new test has failed, or was too long, this check will be red. We don't allow flaky tests, read the doc	✅ success
Install packages	Checks that the built packages are installable in a clear environment	✅ success
Integration tests	The integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests	✅ success
Performance Comparison	Measure changes in query performance. The performance test report is described in detail here. In square brackets are the optional part/total tests	✅ success
Stateful tests	Runs stateful functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	✅ success
Stress test	Runs stateless functional tests concurrently from several clients to detect concurrency-related errors	✅ success
Style check	Runs a set of checks to keep the code style clean. If some of tests failed, see the related log from the report	✅ success
Unit tests	Runs the unit tests for different release types	✅ success
Upgrade check	Runs stress tests on server version from last release and then tries to upgrade it to the version from the PR. It checks if the new server can successfully startup without any errors, crashes or sanitizer asserts	✅ success

vdimir · 2024-09-06T16:10:58Z

Need also to enable setting in integration tests by #69328 However, the resr should be alright

nickitat · 2024-09-17T13:11:20Z

src/Processors/QueryPlan/Optimizations/Optimizations.h

 void optimizePrewhere(Stack & stack, QueryPlan::Nodes & nodes);
 void optimizeReadInOrder(QueryPlan::Node & node, QueryPlan::Nodes & nodes);
 void optimizeAggregationInOrder(QueryPlan::Node & node, QueryPlan::Nodes &);
+void optimizeJoin(QueryPlan::Node & node, QueryPlan::Nodes &);


src/Processors/QueryPlan/Optimizations/optimizeTree.cpp

src/Planner/CollectColumnIdentifiers.cpp

nickitat · 2024-09-17T17:47:20Z

src/Processors/QueryPlan/Optimizations/optimizeJoin.cpp

+        return;
+
+    const auto & join = join_step->getJoin();
+    if (join->pipelineType() != JoinPipelineType::FillRightFirst || !join->isCloneSupported() || typeid_cast<const HashJoin *>(join.get()))


could we allow ConcurrentHashJoin here too?

I enabled isCloneSupported for ConcurrentHashJoin, should work for that implementation as well

nickitat · 2024-09-17T19:51:52Z

src/Processors/QueryPlan/Optimizations/optimizeJoin.cpp

+
+    const auto & table_join = join->getTableJoin();
+    auto kind = table_join.kind();
+    if (table_join.hasUsing()


comment for this if pls

Actually it's a todo, since using implemented so it depends on table orders, can be fixed but a bit of headache

ClickHouse/src/Processors/QueryPlan/Optimizations/optimizeJoin.cpp

Lines 64 to 65 in a457683

/// fixme: USING clause handled specially in join algorithm, so swap breaks it

/// fixme: Swapping for SEMI and ANTI joins should be alright, need to try to enable it and test

src/Interpreters/TableJoin.cpp

nickitat · 2024-09-17T20:22:36Z

src/Processors/QueryPlan/Optimizations/optimizeJoin.cpp

+    join_step->swap_streams = true;
+
+    auto updated_table_join = std::make_shared<TableJoin>(table_join);
+    updated_table_join->swapSides();


it seems it could happen that different nodes could make different optimisation decisions here in a distributed query. afaiu we change output header too. is it actually a problem and should we then maybe take some actions to preserve the header?

Implemented transformations inside JoinStep, so it keeps column order unchanged

Signed-off-by: vdimir <vdimir@clickhouse.com>

src/Interpreters/ConcurrentHashJoin.h

src/Processors/QueryPlan/UnionStep.cpp

tests/clickhouse-test

src/Processors/QueryPlan/Optimizations/optimizeJoin.cpp

vdimir · 2024-11-01T12:28:33Z

Stateless tests (debug, s3 storage) — fail: 1, passed: 6984, skipped: 181 Details

#71366

Algunenano · 2024-11-04T20:09:41Z

src/Core/SettingsChangesHistory.cpp

            {"restore_replace_external_dictionary_source_to_null", false, false, "New setting."},
            {"show_create_query_identifier_quoting_rule", "when_necessary", "when_necessary", "New setting."},
            {"show_create_query_identifier_quoting_style", "Backticks", "Backticks", "New setting."},
+            {"query_plan_join_inner_table_selection", "auto", "auto", "New setting."},


This should go to 24.11

* [GLUTEN-1632][CH]Daily Update Clickhouse Version (20241105) * Fix Build due to ClickHouse/ClickHouse#71261 * Fix Build due to ClickHouse/ClickHouse#68682 --------- Co-authored-by: kyligence-git <gluten@kyligence.io> Co-authored-by: Chang Chen <baibaichen@gmail.com>

…/ClickHouse#71527

* [GLUTEN-1632][CH]Daily Update Clickhouse Version (20241107) * Revert "Fix Build due to ClickHouse/ClickHouse#68682", see ClickHouse/ClickHouse#71527 --------- Co-authored-by: kyligence-git <gluten@kyligence.io> Co-authored-by: Chang Chen <baibaichen@gmail.com>

Resubmit #68682

…lect_inner_table Revert "Resubmit #68682"

Resubmit #68682, att2

robot-clickhouse-ci-2 added the pr-feature Pull request with new product feature label Aug 21, 2024

vdimir changed the title ~~[wip] select inner table for hash join~~ [wip, do not review] select inner table for hash join Aug 21, 2024

vdimir force-pushed the vdimir/join_select_inner_table branch 2 times, most recently from 8276d1d to 78bfc55 Compare August 26, 2024 10:42

vdimir changed the title ~~[wip, do not review] select inner table for hash join~~ select inner table for hash join Aug 29, 2024

vdimir marked this pull request as ready for review August 29, 2024 09:47

alexey-milovidov mentioned this pull request Sep 1, 2024

Add join reordering #52999

Closed

vdimir changed the title ~~select inner table for hash join~~ Automatic selection of hash join inner table Sep 6, 2024

vdimir changed the title ~~Automatic selection of hash join inner table~~ Selection of hash join inner table Sep 6, 2024

nickitat self-assigned this Sep 8, 2024

vdimir requested a review from nickitat September 17, 2024 16:01

nickitat reviewed Sep 17, 2024

View reviewed changes

vdimir force-pushed the vdimir/join_select_inner_table branch 2 times, most recently from 4223def to 3cf2b90 Compare September 26, 2024 12:12

vdimir added 7 commits September 26, 2024 12:12

[wip] select inner table for hash join

b4d7174

fix column not found

12e0b14

fix count() with query_plan_join_inner_table_selection

7605a76

Fix 'auto' join with inner table selection

0598419

setting

2b82db2

join step swap header

da2e6ae

fix stylecheck

35cf3e8

Signed-off-by: vdimir <vdimir@clickhouse.com>

vdimir force-pushed the vdimir/join_select_inner_table branch from 3cf2b90 to 35cf3e8 Compare September 26, 2024 13:39

vdimir added 2 commits September 26, 2024 14:19

Fix header

3ee6fd9

fix stylecheck

fca592a

vdimir force-pushed the vdimir/join_select_inner_table branch from 697017b to fca592a Compare September 26, 2024 14:20

robot-clickhouse and others added 2 commits September 26, 2024 14:30

Automatic style fix

d39d9a8

add ColumnPermuteTransform

9642e6c

vdimir marked this pull request as draft September 27, 2024 09:01

reresolve conflicts

32fe869

vdimir force-pushed the vdimir/join_select_inner_table branch from ede7e1d to 32fe869 Compare October 24, 2024 12:19

fix

a457683

vdimir marked this pull request as ready for review October 25, 2024 09:52

vdimir requested a review from nickitat October 25, 2024 09:52

nickitat approved these changes Oct 28, 2024

View reviewed changes

src/Interpreters/ConcurrentHashJoin.h Show resolved Hide resolved

src/Processors/QueryPlan/UnionStep.cpp Outdated Show resolved Hide resolved

tests/clickhouse-test Outdated Show resolved Hide resolved

src/Processors/QueryPlan/Optimizations/optimizeJoin.cpp Show resolved Hide resolved

finishing

b5e3df9

vdimir enabled auto-merge November 1, 2024 12:28

vdimir disabled auto-merge November 1, 2024 12:31

vdimir added this pull request to the merge queue Nov 4, 2024

Merged via the queue into master with commit 93b1484 Nov 4, 2024

vdimir deleted the vdimir/join_select_inner_table branch November 4, 2024 14:47

robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Nov 4, 2024

Algunenano reviewed Nov 4, 2024

View reviewed changes

Algunenano mentioned this pull request Nov 4, 2024

Fix upgrade check (24.11) #71438

Merged

21 tasks

baibaichen added a commit to Kyligence/gluten that referenced this pull request Nov 5, 2024

Fix Build due to ClickHouse/ClickHouse#68682

b8b75e0

This was referenced Nov 6, 2024

Revert "Selection of hash join inner table" #71527

Merged

Paste join with Group by logical error #71507

Closed

baibaichen added a commit to Kyligence/gluten that referenced this pull request Nov 7, 2024

Revert "Fix Build due to ClickHouse/ClickHouse#68682", see ClickHouse…

640f108

…/ClickHouse#71527

vdimir mentioned this pull request Nov 7, 2024

Non-experimental non-equi join conditions #69910

Merged

21 tasks

github-merge-queue bot pushed a commit that referenced this pull request Nov 25, 2024

Merge pull request #71577 from ClickHouse/vdimir/join_select_inner_table

95b21f4

Resubmit #68682

Algunenano added a commit that referenced this pull request Nov 25, 2024

Revert "Resubmit #68682"

4585a62

github-merge-queue bot pushed a commit that referenced this pull request Nov 25, 2024

Merge pull request #72421 from ClickHouse/revert-71577-vdimir/join_se…

b0cd933

…lect_inner_table Revert "Resubmit #68682"

vdimir added a commit that referenced this pull request Nov 26, 2024

Resubmit #68682, att2

a8b0b74

rschu1ze changed the title ~~Selection of hash join inner table~~ Hash join: automatically choose build side Nov 26, 2024

github-merge-queue bot pushed a commit that referenced this pull request Nov 30, 2024

Merge pull request #72460 from ClickHouse/vdimir/join_select_inner_table

0589253

Resubmit #68682, att2

	/// fixme: USING clause handled specially in join algorithm, so swap breaks it
	/// fixme: Swapping for SEMI and ANTI joins should be alright, need to try to enable it and test

Conversation

vdimir commented Aug 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

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

Documentation entry for user-facing changes

CI Settings (Only check the boxes if you know what you are doing):

Uh oh!

robot-clickhouse commented Aug 21, 2024 • edited by robot-ch-test-poll Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vdimir commented Sep 6, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vdimir commented Nov 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vdimir commented Aug 21, 2024 •

edited

Loading

robot-clickhouse commented Aug 21, 2024 •

edited by robot-ch-test-poll

Loading