Skip to content

Fix incorrect RIGHT join result when using complex ON conditions#94680

Merged
vdimir merged 1 commit intomasterfrom
vdimir/parallel_right_join_flag_per_row
Jan 22, 2026
Merged

Fix incorrect RIGHT join result when using complex ON conditions#94680
vdimir merged 1 commit intomasterfrom
vdimir/parallel_right_join_flag_per_row

Conversation

@vdimir
Copy link
Copy Markdown
Member

@vdimir vdimir commented Jan 20, 2026

Changelog category (leave one):

  • Critical Bug Fix (crash, data loss, RBAC) or LOGICAL_ERROR

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

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Jan 20, 2026

Workflow [PR], commit [dbf74c9]

Summary:

job_name test_name status info comment
BuzzHouse (amd_debug) failure
Logical error: 'Inconsistent AST formatting: the query: (STID: 1941-1bfa) FAIL cidb, issue ISSUE EXISTS

@clickhouse-gh clickhouse-gh bot added pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! labels Jan 20, 2026
@vdimir vdimir force-pushed the vdimir/parallel_right_join_flag_per_row branch from e35f5a3 to dbf74c9 Compare January 21, 2026 09:20
@novikd novikd self-assigned this Jan 21, 2026
SELECT
*
FROM t1
RIGHT JOIN t0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's add queries with LEFT JOIN from the issue as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! I took a look, and queries using LEFT JOIN return the correct results (unless they’re rewritten to RIGHT JOIN depending on query_plan_join_swap_table, what happened in query from the issue). Since this PR is specifically fixing the RIGHT JOIN behavior, adding LEFT JOIN cases wouldn’t actually cover any of the code paths changed here.

If you still think adding LEFT JOIN cases would be useful for completeness, I’m happy to do that in a follow-up PR. That way we can merge this fix without waiting on another CI run, which might be helpful given the upcoming release.

Comment on lines 54 to +60
per_row_flags[columns] = std::vector<std::atomic_bool>(columns->at(0)->size());

/// Mark all rows outside of selector as used.
/// We should not emit them in RIGHT/FULL JOIN result,
/// since they belongs to another shard, which will handle flags for these rows
for (auto & flag : per_row_flags[columns])
flag.store(true);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
per_row_flags[columns] = std::vector<std::atomic_bool>(columns->at(0)->size());
/// Mark all rows outside of selector as used.
/// We should not emit them in RIGHT/FULL JOIN result,
/// since they belongs to another shard, which will handle flags for these rows
for (auto & flag : per_row_flags[columns])
flag.store(true);
/// Mark all rows outside of selector as used.
/// We should not emit them in RIGHT/FULL JOIN result,
/// since they belongs to another shard, which will handle flags for these rows
per_row_flags[columns] = std::vector<std::atomic_bool>(columns->at(0)->size(), true);

@vdimir vdimir added this pull request to the merge queue Jan 22, 2026
Merged via the queue into master with commit b1e9697 Jan 22, 2026
130 of 132 checks passed
@vdimir vdimir deleted the vdimir/parallel_right_join_flag_per_row branch January 22, 2026 09:17
robot-ch-test-poll1 added a commit that referenced this pull request Jan 22, 2026
Cherry pick #94680 to 25.10: Fix incorrect RIGHT join result when using complex ON conditions
robot-clickhouse added a commit that referenced this pull request Jan 22, 2026
robot-ch-test-poll1 added a commit that referenced this pull request Jan 22, 2026
Cherry pick #94680 to 25.11: Fix incorrect RIGHT join result when using complex ON conditions
robot-clickhouse added a commit that referenced this pull request Jan 22, 2026
robot-ch-test-poll1 added a commit that referenced this pull request Jan 22, 2026
Cherry pick #94680 to 25.12: Fix incorrect RIGHT join result when using complex ON conditions
robot-clickhouse added a commit that referenced this pull request Jan 22, 2026
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jan 22, 2026
robot-clickhouse added a commit that referenced this pull request Jan 22, 2026
Cherry pick #94680 to 25.3: Fix incorrect RIGHT join result when using complex ON conditions
robot-clickhouse added a commit that referenced this pull request Jan 22, 2026
robot-clickhouse added a commit that referenced this pull request Jan 22, 2026
Cherry pick #94680 to 25.8: Fix incorrect RIGHT join result when using complex ON conditions
robot-clickhouse added a commit that referenced this pull request Jan 22, 2026
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Jan 22, 2026
clickhouse-gh bot added a commit that referenced this pull request Jan 22, 2026
Backport #94680 to 25.12: Fix incorrect RIGHT join result when using complex ON conditions
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Jan 22, 2026
vdimir added a commit that referenced this pull request Jan 23, 2026
Backport #94680 to 25.10: Fix incorrect RIGHT join result when using complex ON conditions
vdimir added a commit that referenced this pull request Jan 23, 2026
Backport #94680 to 25.11: Fix incorrect RIGHT join result when using complex ON conditions
vdimir added a commit that referenced this pull request Jan 23, 2026
Backport #94680 to 25.8: Fix incorrect RIGHT join result when using complex ON conditions
@nickitat nickitat added the post-approved Approved, but after the PR is merged. label Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

post-approved Approved, but after the PR is merged. pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR 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.

Concurrent LEFT JOIN different result with query_plan_filter_push_down value

6 participants