Fix parallel_hash global JOIN limit not enforced

[m-selmi]

Closes: #109400
Related: #108938

#108938 made getTotalRowCount / getTotalByteCount lock-free by using atomics and updating them at the end of addBlockToJoin.
This means addBlockToJoin could read a stale state of the atomics and not throw even though the limit was exceeded.
This PR adjusts the accounting to ensure each thread in addBlockToJoin sees an up-to-date state.

Changelog category (leave one):

Bug Fix (user-visible misbehavior in an official stable release)

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

Fixed the global max_rows_in_join / max_bytes_in_join limit not being enforced for the parallel_hash join algorithm, where a query with join_overflow_mode = 'throw' could silently succeed instead of raising SET_SIZE_LIMIT_EXCEEDED.

Version info

• Merged into: 26.7.1.717 (included in 26.7 and later)

[clickhouse-gh]

Workflow [PR], commit [029f1ac]

Summary: ✅

---

AI Review

Summary

The new global accounting in ConcurrentHashJoin looks like the right fix for the stale-snapshot race that let parallel_hash miss the global JOIN limit on ARM, and I do not see the earlier code-level correctness issues in the current head anymore. I cannot approve it yet, though, because the regression coverage still does not prove the same path under join_overflow_mode = 'break', and the PR metadata classifies a stable user-visible bug fix as an Improvement.

PR Metadata

• Changelog category is not correct. This PR fixes user-visible misbehavior in the stable 26.7 line (parallel_hash could ignore max_rows_in_join / max_bytes_in_join after Reduce lock-contention from getTotalByteCount in ConcurrentHashJoin #108938), so it should be Bug Fix, not Improvement.
• Changelog entry is required for Bug Fix, and the current entry is specific enough to keep after changing the category.
• Exact replacement:

### Changelog category (leave one):
Bug Fix

Tests

• ⚠️ [src/Interpreters/ConcurrentHashJoin.cpp:353] The changed accounting path is the shared global limit check for both join_overflow_mode = 'throw' and join_overflow_mode = 'break', but the only permanent regression coverage in tests/queries/0_stateless/04061_spilling_hash_join_overflow_limits.sql still exercises only throw. The previous revision of this PR already showed that a throw-only safety net can leave BREAK silently returning the full result, so this still needs a focused stateless test that forces join_algorithm = 'parallel_hash' with low max_rows_in_join / max_bytes_in_join and asserts truncation under join_overflow_mode = 'break'.

Final Verdict

• Status: ⚠️ Request changes
• Minimum required actions:
  • Add focused parallel_hash regression coverage for join_overflow_mode = 'break'.
  • Change the Changelog category to Bug Fix.

[alexey-milovidov]

Updating the branch to fix the 04001_join_reorder_through_expression test, which was broken today in master.

[clickhouse-gh]

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	85.70%	85.70%	+0.00%
Functions	92.70%	92.70%	+0.00%
Branches	77.90%	77.90%	+0.00%

Changed lines: Changed C/C++ lines covered: 43/44 (97.73%) · Uncovered code

Full report · Diff report

[clickhouse-gh]

parallel_hash is documented to honor both THROW and BREAK for join_overflow_mode, but the regression coverage here still only exercises THROW. The previous revision of this fix already showed that a throw-only safety net can leave BREAK returning the full result after the global limit is crossed, so I think this needs a focused stateless test that forces parallel_hash with low max_rows_in_join / max_bytes_in_join and asserts truncation under join_overflow_mode = 'break'.

[m-selmi]

@antaljanosbenjamin I changed the accounting to make the limit check in addBlockToJoin work more consistently with how it was before, would be great if you take another quick look