Workflow [PR], commit [186483a]

Summary: ✅

AI Review

Summary

This PR fixes a real correctness/safety issue in bech32Encode by validating the user-provided witness version before narrowing to uint8_t, preventing truncation from bypassing the [0, 16] check and avoiding potential out-of-bounds indexing in bech32::encode. The code change itself looks correct and surgical. I found one important gap: test coverage should include wider unsigned input types accepted by the function signature.

Findings

⚠️ Majors

• [src/Functions/bech32.cpp:292] The fix validates a full-width value before narrowing, but regression coverage currently exercises only UInt8-like literals. Because the function accepts any native unsigned type, add a test with wider types (for example toUInt16(272) / toUInt64(1000)) and assert BAD_ARGUMENTS to lock in the intended behavior.

Tests

• ⚠️ Add a stateless query case proving that out-of-range values from wider unsigned types (UInt16/UInt32/UInt64) are rejected with BAD_ARGUMENTS. This is the key regression scenario for the truncation bug class this PR addresses.

ClickHouse Rules

Final Verdict

• Status: ⚠️ Request changes
• Minimum required actions:
  • Add regression tests for out-of-range witness versions passed as wider unsigned types (not only UInt8-range literals).

The Fast test (arm_darwin) failure is unrelated to this PR — the fix is in #102184.

Looks good, but it should either be "Not for changelog" (I don't see this as a real bug), or we need to add/update a tests to reflect the change

I changed it to "Not for changelog"

The Hung check failure is fixed by #102008 and #102010, let's update the branch

LLVM Coverage Report

Changed lines: 100.00% (17/17) | lost baseline coverage: 1 line(s) · Uncovered code

Full report · Diff report

Hi — this PR may need backporting to 26.3 (LTS), 26.2, 26.1, 25.8 (LTS), but no backport label was found.

Affected code: EncodeToBech32Representation::execute in src/Functions/bech32.cpp (witness_version validation) — introduced in 25.8.

Why: The buggy validation introduced by PR #96671 was backported to 25.8, 25.11, 25.12, and 26.1 (verified via git log --all --diff-filter=A -- tests/queries/0_stateless/03915_bech32_invalid_witness_version.sql). All currently supported branches (25.8 LTS, 26.1, 26.2, 26.3 LTS) contain the original bech32Encode validation with the truncation bug, so the fix should be backported to each.

If this should be backported, consider adding pr-must-backport or a version-specific label (e.g. v26.3-must-backport). Ignore this if backporting is not applicable.