Will there be any keyword/control over ANY/ALL behavior?

No, ANY/ALL is controlled by the JOIN strictness in the query itself, not by a separate LOOKUP INDEX option.

Workflow [PR], commit [eca1052]

Summary: ❌

• Performance Comparison: Performance dashboard

AI Review

Summary

This PR adds experimental table-wide LOOKUP INDEX support for MergeTree, covering table_set membership lookups and table_join direct joins. The current implementation still has one unresolved access-policy issue in the table_join fast path: it can read and cache physical columns outside the query's checked column set.

Findings

• ⚠️ Majors
  • [src/Planner/PlannerJoins.cpp:1139] table_join lookup cache construction can read and cache physical columns the user was not granted SELECT on. The normal table-expression planning checks only getSelectedColumnsNames, but MergeTreeData::tryGetLookupJoin reads every physical column via getAllPhysicalColumnsForLookupJoin, so a join that only needs the key and one payload column can still populate a shared entity containing protected columns.
  • Suggested fix: either build the entity from the required right-side columns plus lookup keys, or perform SELECT access validation on the full lookup_columns set before tryGetLookupJoin can read and cache them.

Tests

• ⚠️ Add a focused RBAC stateless test for table_join: grant a user SELECT only on the join key and one payload column, deny another physical column, enable allow_experimental_lookup_index, and verify the lookup path is not allowed to read or cache the denied column.

Final Verdict

Status: ⚠️ Request changes

Minimum required action: fix the table_join column-access gap and cover it with the RBAC regression above.

All committers have signed the CLA.

Picked up the PR to address the open AI review finding.

Fix (192b203410a)

The single ⚠️ Major / unresolved thread was the table_join lookup fast path being selected when the right table expression reads ALIAS columns. The lookup entity MergeTreeTableJoinEntity is built from physical columns only (getAllPhysicalColumnsForLookupJoin), so DirectKeyValueJoin requesting a computed alias from it raised Cannot find column ... in table lookup cache instead of falling back.

tryGetLookupJoinStorage now falls back to the regular join path when the right table expression reads alias columns (getAliasColumnExpressions() non-empty). Added 04401_lookup_index_alias_column (alias select computes concat(val, '!'); mixed physical+alias select; EXPLAIN confirms no DirectKeyValueJoin when an alias is read but the fast path is kept for physical-only selects) plus a table_join limitations note in lookupindexes.md.

Verification

Built locally and ran all lookup-index stateless tests against a server — 04063, 04064, 04068, 04069, 04244, 04312, 04360, and the new 04401 all pass. The thread is resolved.

CI

The red checks on the reviewed commit are all unrelated master-wide flakes, confirmed against CIDB:

• Stress (amd_tsan): Cannot start clickhouse-server (on master and 16+ PRs on the same day) and Unexpected exception in refresh scheduling (Logical error: 'Unexpected exception in refresh scheduling' (STID: 2508-3e7b) #106737, 129 failures / 100 PRs in 14 days);
• AST fuzzer (arm_asan_ubsan): Block structure mismatch in A stream in UnionStep::updateOutputHeader (occurs on master PR=0 and ~19 PRs/day);
• Performance Comparison (arm_release): unstable/slower noise across unrelated tests (number_formatting_formats, point_in_polygon_*, replaceRegexp_*, …); the PR's own lookup_index_* perf tests are not among the failures.

Not done (left for you)

• I did not re-merge master (merge base is ~2 days old, well under a week, and the reds are flakes; the branch is MERGEABLE). Re-merging the full 659-commit delta into the heavily-churned join planner code seemed riskier than helpful.
• The 2026-06-18 master merge left several stateless tests with duplicate number prefixes (04063, 04064, 04068, 04069, 04244, 04312 each now collide with unrelated master tests at the same number). This does not break CI (tests run by full name) but should be renumbered as part of the final master merge before merge.
• No self-approve / merge.

Picked up the PR to address the two open AI review findings on the table_join lookup fast path, plus a build break from the latest master merge.

Build fix (c679f483cc4)

getInMemoryMetadataPtr(ContextPtr, bool) now returns a StorageMetadataHandle whose rvalue operator-> is deleted to prevent dereferencing a temporary. hasLookupSetIndex / hasLookupJoinIndex called ->getLookupIndices() directly on the returned temporary, selecting the deleted overload and failing to compile. Bound the handle to a named local first.

Review fixes (68a76542eb2)

• Subcolumns / virtual columns — the previous fallback only rejected ALIAS payload columns, but the analyzer can map a right-side identifier to a subcolumn (e.g. m.keys) or a virtual column (e.g. _part), which the physical-only lookup entity cannot serve, so DirectKeyValueJoin raised Cannot find column ... in table lookup cache. tryGetLookupJoinStorage now falls back unless every mapped right-side column is in the lookup entity column set (getColumns(AllPhysical), the same set getAllPhysicalColumnsForLookupJoin uses).
• Join size limits — the lookup build path never honored max_rows_in_join / max_bytes_in_join. The fast path now declines when either limit is active so the regular join enforces them.

New tests 04408_lookup_index_table_join_subcolumn and 04409_lookup_index_table_join_size_limits.

Flaky test fix (18a4f6abb07)

04401_lookup_index_alias_column did not pin parallel replicas, so when the harness randomly enabled enable_parallel_replicas the lookup fast path was disabled (!canUseParallelReplicasOnInitiator()) and the physical-only assertion failed intermittently. Pinned enable_parallel_replicas = 0 / max_parallel_replicas = 1 / serialize_query_plan = 0, matching the other lookup index tests.

Verification

Built locally and ran the full lookup index suite (04063, 04064, 04068, 04069, 04244, 04312, 04360, 04401, 04408, 04409) against a server — all pass, repeatedly under the randomized settings. Both review threads are resolved.

Pushed 18a4f6abb07..96ae2957a0f (3 commits) fixing the Fast-test failures from the last manual approve run and the remaining validation thread.

What failed and why

The previous run went red on six table_join lookup tests (04064, 04068, 04360, 04401, 04408, 04409) — all result differs with reference, DirectKeyValueJoin expected 1, got 0 — plus 02995_new_settings_history.

Root cause of the six: the earlier fix that declines the table_join lookup fast path when max_rows_in_join != 0 || max_bytes_in_join != 0. The standard stateless config tests/config/users.d/limits.yaml sets both to 10G ("so it will not limit anything, but we test code around it"), so under the Fast-test config the fast path declined universally and DirectKeyValueJoin was absent everywhere.

Commits

1. Reject compound subcolumn keys in LOOKUP INDEX validation — validateTableLookupKeys now rejects identifier->compound(), so LOOKUP INDEX idx (m.keys) TYPE table_join is rejected instead of being accepted as an unusable index built from top-level physical columns. A quoted dotted name (`a.b`) is a single-part identifier and is still accepted. New test 04411_lookup_index_compound_key_rejected. (Resolves the open MergeTreeIndexTableLookup.cpp thread.)
2. Reset join size limits in the table_join lookup tests — SET max_rows_in_join = 0, max_bytes_in_join = 0 at session level in the tests that assert the fast path fires, so they exercise it regardless of the profile ceiling. 04409 keeps its per-query non-zero limits for the decline assertions.
3. Record allow_experimental_lookup_index under 26.7 in SettingsChangesHistory.cpp — master advanced to 26.7, and 02995 requires new settings (vs the 26.6.1 baseline) to be recorded for a version > 26.6.

Verification (local, real server)

Reproduced the Fast-test condition by mirroring limits.yaml (all four max_rows/bytes_in_join/set = 10G) in the default profile and adding a parallel_replicas cluster:

• Full 11-test lookup suite passes with randomization off and 5/5 with randomization on.
• 02995 verified manually (no-cpu-aarch64, can't run via the harness on ARM): allow_experimental_lookup_index is absent from the 26.6.1 baseline and recorded for 26.7, so the new-setting check is empty.

Design question to confirm (not changed here)

The decline-on-non-zero behavior means the table_join fast path is disabled whenever any join size ceiling is set, including the standard 10G test profile and any production profile that sets max_rows_in_join / max_bytes_in_join — the same applies to table_set with max_rows_in_set / max_bytes_in_set. That is fail-safe (it never silently ignores a user-set limit), but it also means the feature is off under most non-trivial profiles. The alternative is to enforce the limit during the lookup build and keep the fast path enabled. I've kept the current fail-safe behavior; whether to switch to enforce-in-build is a design call for you to make.

Picked up the PR after the manual approve CI run on 96ae2957a0f.

CI run result

The Fast-test / stateless suite is green — all lookup-index tests pass, confirming the size-limit and validation fixes from the previous run landed correctly. The remaining red checks are all unrelated master-wide flakes (confirmed against CIDB):

• Stress (amd_tsan) — Hung check failed, possible deadlock found (Hung check failed, possible deadlock found #107941; the log tail is an unrelated 00534_functions_bad_arguments10.sh).
• Stress (arm_ubsan) — Coordination mode mismatch for stream ... (STID 6117-68e9), a recurring parallel-replicas stress flake that hits master itself and ~30 unrelated PRs over the last month.
• Performance Comparison (arm_release) — sort_patterns unstable / noise, unrelated to the lookup index.
• CH Inc sync, Mergeable Check, PR — internal sync / meta checks.

Fix this run (0fde27aa63a)

The CI re-review raised a new SELECT-access finding on the table_set lookup fast path, and it is real. tryGetLookupSetFromTableExpression builds the Set directly from storage and registers it with sets.addFromStorage, so the right-hand table is never planned and the regular path's SELECT access check (checkAccessRights / checkAccessRightsForSubquery in PlannerJoinTree) was skipped. A user holding SELECT only on the left table could probe a dimension table's keys through IN table — but only when the fast path actually fires, i.e. with no active max_rows_in_set / max_bytes_in_set limit (the standard config's high ceiling makes the fast path decline, which is why the existing tests did not surface it).

The fast path now performs the same SELECT access check on the lookup key columns before using the Set, in both the direct IN table and the trivial IN (SELECT key FROM table) branches.

Verification (server with RBAC, key columns granted vs. not): with the fast path firing (max_rows_in_set = 0), an unauthorized user is now denied — the denial originates from CollectSets.cpp itself, confirming the fast path enforces it — while an authorized user gets correct results; without the fast path the behaviour is unchanged. New test 04412_lookup_index_table_set_access covers both IN forms with and without access; existing 04063 / 04069 / 04244 / 04312 still pass. The review thread is resolved.

Did not re-merge master: the branch is ~2 days behind and the red checks are flakes, not staleness.

Addressed the two ⚠️ Request changes required actions from the AI review on f07b5390c56:

1. table_set access check ordering (aa73fffc0cd): in tryGetLookupSetFromTableExpression, the SELECT access check on the lookup key columns now runs before storage->tryGetLookupSet(...) in both branches (direct IN table and the trivial IN (SELECT key FROM table) subquery). Previously the check ran after the read, so an unauthorized user — while denied the final result — could still force a read and lookup-Set cache build of a table they cannot SELECT. The check is cheap and idempotent: when no matching table_set index exists, tryGetLookupSet returns early anyway and the regular IN plan performs the same check, so there is no behavior change on the fallback path. Access enforcement stays covered by 04412_lookup_index_table_set_access.

2. Docs example (eca1052f527): the Using table_set example built dim with id, subid, value but used (id, subid) IN dim. The direct IN table form builds the right-hand set from all of the table's columns, so that both failed with a column-count mismatch and never matched the two-column LOOKUP INDEX idx_set (id, subid). The direct example now uses a key-only dim_keys table so it is executable and actually uses the index; the trivial-subquery form IN (SELECT id, subid FROM dim) is shown for dimension tables that also store non-key columns.

CollectSets.cpp was verified to compile cleanly against the current (post-merge) headers. Both review threads are resolved.

The remaining red checks are unrelated master-wide flakes, not caused by this PR (which is SELECT-path only): Stress arm_asan_ubsan Hung check failed (#107941); Upgrade check Error message in clickhouse-server.log = a leftover background mutation Cannot parse string 'x' as UInt64 in MutatePlainMergeTreeTask (unrelated to lookup indexes); and Performance Comparison entries that the perf report classifies as unstable/noise (No significant performance changes detected). All Fast/stateless tests, including the lookup tests, pass.

LLVM Coverage Report

Changed lines: Changed C/C++ lines covered by tests: 941/1112 (84.62%) | Lost baseline coverage (was covered on master, now uncovered in this PR): 44 line(s) · Uncovered code

Full report · Diff report