Workflow [PR], commit [c665f95]

Summary: ✅

AI Review

Summary

This PR removes the allow_experimental_alias_table_engine gate and documents Alias as non-experimental, while keeping the setting obsolete for parsing compatibility and adding a stateless coverage test. I found one blocker: the PR promotes Alias to production even though the known multi-block deduplication row-loss bug in Alias inserts remains unresolved in current code.

Findings

❌ Blockers

• [src/Storages/StorageAlias.cpp:328] Dropping the experimental gate exposes users to an unresolved correctness bug in Alias writes: AliasSink::consume still builds a new InterpreterInsertQuery per chunk, which can collapse dedup hashes when insert_deduplication_token is set and silently lose blocks 2..N on multi-block inserts. This is tracked in #103118; fix PR #103246 is still open/draft.
  Suggested fix: keep Alias gated until the write-path fix lands, or include the onStart/single-pipeline fix plus regression coverage in this PR.

Final Verdict

Status: ⚠️ Request changes

Minimum required actions:

1. Land the AliasSink multi-block deduplication fix (or equivalent) with regression coverage.
2. Keep the Alias engine experimental until that fix is merged, or merge both changes together.

Blocked by #103118

@groeneai, investigate the failure: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=103488&sha=e19f797220055d6bf832847454ee60c545557f20&name_0=PR&name_1=Unit%20tests%20%28tsan%2C%20function_prop_fuzzer%29 and provide a fix in a separate PR.

The FunctionsStress.stress failure is a function-prop fuzzer non-determinism finding (caseWithExpression over Date32/DateTime64), unrelated to this PR which only flips Alias table engine out of experimental. It is tracked in #104877.

@groeneai, the remaining CI failures look unrelated to this PR:

• FunctionsStress.stress / AllTests in Unit tests (asan_ubsan, function_prop_fuzzer) and Unit tests (msan, function_prop_fuzzer) — non-determinism in isDistinctFrom (String vs constant FixedString) and caseWithExpression. Tracked in FunctionsStress.stress #104877; fix already in progress in Fix more non-deterministic function results found by the functions stress test #104858, which touches FunctionsComparison.h to add a stringVectorConstantFixedString path that respects zero-padded FixedString semantics — exactly the pattern that fails here.
• 01548_query_log_query_execution_ms in Stateless tests (amd_msan, WasmEdge, parallel, 2/2) — timeout under MSan/WasmEdge; the in-job rerun passed ("All reruns passed. The failure is not reproducible (likely a transient issue)."). The test exercises query_log / query_execution_ms and is unrelated to the Alias table engine. Please investigate and provide a fix in a separate PR if needed.

@alexey-milovidov — confirmed, both items are tracked under in‑flight fixes:

1. FunctionsStress.stress (isDistinctFrom(String, FixedString) + caseWithExpression) → #104877

The umbrella covers two distinct code paths, each fixed by a separate PR:

• isDistinctFrom / isNotDistinctFrom (String vector vs constant FixedString), plus the if(...) / nullIf(...) cross-type variants → fixed by Fix more non-deterministic function results found by the functions stress test #104858 (@Algunenano, OPEN). Adds stringVectorConstantFixedString in FunctionsComparison.h so String-vector × FixedString-constant goes through memcmpSmallLikeZeroPaddedAllowOverflow15 (zero-padded comparison), aligning the constant path with the String vs FixedString vector path; adds castForIf in src/Functions/if.cpp to keep the const-cond fast path consistent with the non-const FixedStringSource → StringSink path so const-folding the condition cannot flip the result. This is exactly the pattern you pointed at on this PR.
• caseWithExpression / transform (FixedString / Enum / Date32 / DateTime64) → fixed by Fix transform default-branch conversion bug (FixedString / Enum) #104804 (groeneai, OPEN). The cache->default_column was built via the Field round-trip which loses information for some destination types (different from castColumn semantics). The PR forwards the default through castColumn + cloneResized(1) at the wrapper layer, so all destination type variants converge.

Both PRs target the same root pattern (FixedString NUL‑padding handling differs across const-vs-non-const dispatch and across the Field round-trip), and together cover every shape seen so far in #104877 (9 distinct function shapes + sub-family A vs B classification, latest tracker in the issue thread).

2. 01548_query_log_query_execution_ms (amd_msan WasmEdge timeout) → tracked under #104866

This belongs to the chronic Stateless tests (amd_msan, WasmEdge, *) SYSTEM FLUSH LOGS query_log / per-test timeout family (11+ distinct tests across the lane in the last 14 days, several with master hits). Root cause was identified by @Algunenano on #104866 (2026-05-14T13:54:22Z): the x86v3 baseline ISA flip puts __msan_chain_origin on a hot path where LLVM inserts vzeroupper around every AVX2/SSE boundary, pushing wall‑time over the 180s flush threshold. His patch is incoming. The in‑job rerun pass you observed on this PR (All reruns passed. The failure is not reproducible (likely a transient issue).) matches that root cause — the lane is borderline, so any same‑run reschedule clears it.

No follow‑up action requested on the Alias PR — both workstreams are owned and active.

LLVM Coverage Report

Changed lines: 100.00% (22/22) · Uncovered code

Full report · Diff report