Skip to content

Fix LazyMaterializingTransform lazy chunk mismatch with vector search rescoring#111003

Merged
shankar-iyer merged 1 commit into
ClickHouse:masterfrom
groeneai:groeneai/fix-lazymat-vector-rescoring-4770-4443
Jul 20, 2026
Merged

Fix LazyMaterializingTransform lazy chunk mismatch with vector search rescoring#111003
shankar-iyer merged 1 commit into
ClickHouse:masterfrom
groeneai:groeneai/fix-lazymat-vector-rescoring-4770-4443

Conversation

@groeneai

@groeneai groeneai commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Related: #97223

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):

Fix a LOGICAL_ERROR (LazyMaterializingTransform: Number of rows in lazy chunk N does not match number of offsets M) that could happen for vector search queries with vector_search_with_rescoring = 1 combined with lazy materialization when several distances tie.

Description

Lazy materialization builds its lazy read from ReadFromMergeTree::getParts(), which copies the parts together with the use_vector_search_result_filter read hint that vector search rescoring sets on them. The lazy read then re-applied the vector index candidate filter on top of the rows it had already selected by global row index. When several distances tie, the vector index candidate set can differ from the rows the LIMIT kept, so the extra filter dropped a selected row and the lazy chunk ended up shorter than the offsets, tripping the assertion in prepareLazyChunk.

The lazy read must re-fetch exactly the rows the main read selected and must not re-filter by vector candidates, so use_vector_search_result_filter is cleared on the parts handed to the lazy read. The main read keeps the hint and is unchanged.

Reproducer (STID 4770-4443, found by the AST fuzzer, e.g. https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=108846&sha=0d5e67f1a19eeb4051f2ca77cf8033c9547f5187&name_0=PR&name_1=AST%20fuzzer%20%28amd_debug%2C%20targeted%29):

CREATE TABLE tab (id UInt64, payload String, vec Array(Float32),
  INDEX idx_vec vec TYPE vector_similarity('hnsw', 'L2Distance', 2) GRANULARITY 100000000)
ENGINE = MergeTree ORDER BY id
SETTINGS index_granularity = 4, min_bytes_for_wide_part = 0, max_bytes_to_merge_at_max_space_in_pool = 1;
INSERT INTO tab SELECT number, repeat('x', 32), [toFloat32(number), 0] FROM numbers(12);
WITH [1000.0001, 3.4028234663852886e38] AS reference_vec
SELECT payload FROM tab ORDER BY L2Distance(vec, reference_vec) LIMIT 3
SETTINGS vector_search_with_rescoring = 1;

Version info

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

… rescoring

Lazy materialization built its lazy read from ReadFromMergeTree::getParts(),
copying the parts together with the use_vector_search_result_filter read hint set
for vector_search_with_rescoring=1. The lazy read then re-applied the vector index
candidate filter on top of the rows it already selected by global row index. When
several distances tie, the vector index candidate set can differ from the rows the
LIMIT kept, so the extra filter dropped a selected row and the lazy chunk ended up
shorter than the offsets, tripping a LOGICAL_ERROR in prepareLazyChunk.

Clear use_vector_search_result_filter on the parts handed to the lazy read: it must
re-fetch exactly the rows the main read selected and must not re-filter by vector
candidates. The main read keeps the hint and behaves as before.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@groeneai

Copy link
Copy Markdown
Contributor Author
Pre-PR validation gate (click to expand)
# Question Answer
a Deterministic repro? Yes. WITH [1000.0001, 3.4028234663852886e38] AS r SELECT payload FROM tab ORDER BY L2Distance(vec, r) LIMIT 3 SETTINGS vector_search_with_rescoring = 1 on the schema in the PR body raises the LOGICAL_ERROR on the first run (SIGABRT in debug).
b Root cause explained? Yes. Lazy materialization builds its lazy read from getParts(), copying the use_vector_search_result_filter hint. The lazy read then re-applies the vector-index candidate filter on top of the rows it already selected by global row index. When distances tie, the candidate set differs from the LIMIT-kept rows, so a selected row is dropped and the lazy chunk is shorter than the offsets.
c Fix matches root cause? Yes. Clears use_vector_search_result_filter on the parts handed to the lazy read, so it re-fetches exactly the selected rows without re-filtering.
d Test intent preserved / new tests added? Yes. Extended 04489_vector_search_rescoring_lazy_materialization with the tied-distance case; it crashes without the fix and passes with it. Existing assertions unchanged.
e Both directions demonstrated? Yes. Without fix: SIGABRT on run 1 (and the extended test crashes). With fix: 40/40 repro runs return the correct 3 rows, the extended test passes, all 10 vector-search rescoring/lazy tests pass, results match the non-lazy and brute-force baselines.
f Fix is general across code paths? Yes. use_vector_search_result_filter is consumed at exactly two reader sites, both gating only the in-granule row filter; getParts() is the single lazy-read source.
g Fix generalizes across inputs? Yes. The clear is unconditional for every lazy read, so it covers all reference vectors / types / LIMIT values, not just the degenerate repro. The lazy-materialization optimization is preserved (LazilyReadFromMergeTree still in the plan).
h Backward compatible? Yes. No setting, format, protocol, or ABI change; internal query-plan fix only.
i Invariants and contracts preserved? Yes. The hint only ever enables the vector row filter, never mark/range selection (the lazy read drives ranges via filterRangesAndFillRows). The hint is only set for vector_search_with_rescoring = 1 and non-FINAL, so the non-rescoring lazy+vector path and FINAL are untouched.

Session id: cron:clickhouse-author-slot-0:20260719-142200

@groeneai

Copy link
Copy Markdown
Contributor Author
Internal second-model review — adjudication log (click to expand)

Pre-publication review by an independent model (engine: codex; 1 full pass + 1 delta recheck).

# Sev Finding Verdict Evidence / action
1 ⚠️ .meta.json (task-runner metadata) would be committed DISAGREE Not in the commit: git cat-file -t HEAD:.meta.json fails (absent from tree), git status shows it ?? (untracked), git show --stat HEAD lists exactly the 3 intended files. Workspace file, never staged.
2 💡 Comment called the failure an "assertion" AGREE Reworded to "raising a LOGICAL_ERROR in prepareLazyChunk" (it is a caught exception, not an assertion). Comment-only; Build ID unchanged; re-verified repro + regression test.
3 Delta recheck: "functional fix / regression test missing" DROPPED Artifact of the delta scope — the recheck diffed only the 1-line comment amend, so it did not see the flag-clear or the test. Both are present in HEAD (git show --stat HEAD = 3 files; the use_vector_search_result_filter = false line and the extended 04489 test are in the commit) and were reviewed in the full pass.
4 ⚠️ Delta recheck: remove .meta.json DROPPED Duplicate of finding 1 (already DISAGREE with evidence).

Severity: ❌ blocker / ⚠️ major / 💡 nit. DISAGREE verdicts carry recorded evidence and are terminal per finding. Findings on hunks unchanged by the fix round are auto-dropped.

Session id: cron:clickhouse-author-slot-0:20260719-142200

@shankar-iyer shankar-iyer self-assigned this Jul 19, 2026
@shankar-iyer shankar-iyer added the can be tested Allows running workflows for external contributors label Jul 19, 2026
@groeneai

Copy link
Copy Markdown
Contributor Author

cc @novikd @shankar-iyer — could you review this? It fixes a LOGICAL_ERROR in prepareLazyChunk for vector_search_with_rescoring = 1 + lazy materialization: the lazy read inherited the use_vector_search_result_filter hint via getParts() and re-applied the vector-index candidate filter over the rows it had already selected by global row index, so under tied distances a selected row was dropped. The fix clears that hint on the lazy read's parts (the main read is unchanged).

@clickhouse-gh

clickhouse-gh Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [3d0f860]

Summary:


AI Review

Summary

This PR fixes the lazy-materialization/vector-search rescoring interaction by clearing use_vector_search_result_filter on the copied part ranges that drive the lazy reread, so the lazy branch re-fetches the rows selected by LazyMaterializingRows instead of intersecting them with the vector-index candidate set a second time. The added regression in 04489_vector_search_rescoring_lazy_materialization covers the tied-distance case that previously raised the LOGICAL_ERROR, and I did not find any remaining sibling-path gap in the current code.

Final Verdict

Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Jul 19, 2026
@clickhouse-gh

clickhouse-gh Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 86.20% 86.30% +0.10%
Functions 92.10% 92.10% +0.00%
Branches 78.20% 78.30% +0.10%

Changed lines: Changed C/C++ lines covered: 8/8 (100.00%) · Uncovered code

Full report · Diff report

@groeneai

Copy link
Copy Markdown
Contributor Author

CI finish ledger — 3d0f860

Every failure below has an owner: a fixing PR (ours or external), or a full-effort fix task whose fixing-PR link will be posted here when it opens.

Check / test Reason Owner / fixing PR
Integration tests (amd_tsan, 3/6) / test_replicated_database::test_replicated_table_structure_alter flaky (100 master + 15 PRs/30d; Replicated-DB DDL-log recovery race, unrelated to this PR) #110030 (ours, open)
Stateless (amd_tsan, s3 storage, parallel, 2/3) / 00909_kill_not_initialized_query flaky (7 PRs, 0 master/30d; KILL-QUERY timing race under tsan+s3) a fix task is created (investigating at full effort, fixing PR link to follow here)
Stateless (amd_tsan, s3 storage, parallel, 2/3) / Scraping system tables infra (minio_audit_logs dump 600s timeout via clickhouse-local on s3) a fix task is created (investigating at full effort, fixing PR link to follow here)

This PR's own diff (LazyMaterializingTransform vector-rescoring fix in optimizeLazyMaterialization.cpp + one vector-search test) is unrelated to all three failures; none is PR-caused.

Session id: cron:our-pr-ci-monitor:20260719-230000

@shankar-iyer
shankar-iyer added this pull request to the merge queue Jul 20, 2026
Merged via the queue into ClickHouse:master with commit 6bf231c Jul 20, 2026
350 of 353 checks passed
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 20, 2026
alexey-milovidov added a commit that referenced this pull request Jul 20, 2026
Pulls in the fix for the unrelated stress-test failure `LazyMaterializingTransform: Number of rows in lazy chunk A does not match number of offsets B` (fixed on master by #111003).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

can be tested Allows running workflows for external contributors pr-bugfix Pull request with bugfix, not backported by default 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.

3 participants