Fix exception in updateFormatPrewhereInfo when only row-level filter is set#100361
Merged
alexey-milovidov merged 8 commits intomasterfrom Mar 29, 2026
Merged
Fix exception in updateFormatPrewhereInfo when only row-level filter is set#100361alexey-milovidov merged 8 commits intomasterfrom
updateFormatPrewhereInfo when only row-level filter is set#100361alexey-milovidov merged 8 commits intomasterfrom
Conversation
…er` is set When `row_level_filter` parameter was added to `updateFormatPrewhereInfo`, the assertion was not updated — it still required `prewhere_info` to be non-null. This caused a logical error when querying object storage (S3, URL, etc.) tables with a row-level security policy but no PREWHERE. Additionally, `row_level_filter` was never saved into the new `ReadFromFormatInfo`, so it would be lost. https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=99537&sha=e2a32dfbb945f62e07bd3f66c206a71b584b96b1&name_0=PR&name_1=Stress%20test%20%28amd_debug%29 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
This test verifies that row policies work correctly on URL tables with Parquet format (which supports PREWHERE). Previously, this would trigger "Logical error: 'prewhere_info'" because only `row_level_filter` was set. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Contributor
|
Workflow [PR], commit [224b120] Summary: ✅ AI ReviewSummaryThis PR fixes ClickHouse Rules
Final Verdict
|
…and prewhere are both active When a table has a row-level security policy (`row_level_filter`) and the optimizer later pushes a WHERE clause into PREWHERE, `updateFormatPrewhereInfo` was called twice: first in `read` (for the row_level_filter) and then in `updatePrewhereInfo` (for both). The guard `if (info.prewhere_info || info.row_level_filter)` rejected the second call because `row_level_filter` was already stored. The fix: - Only guard against duplicate `prewhere_info` (the truly non-idempotent part). - When `row_level_filter` was already applied in a previous call, skip re-applying it and only apply the new `prewhere_info` on top. Also add a `count()` query to the regression test to better exercise the `need_only_count` code path under row-level filtering. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Contributor
LLVM Coverage Report
Changed lines: 100.00% (8/8) · Uncovered code |
mkmkme
pushed a commit
to Altinity/ClickHouse
that referenced
this pull request
Mar 30, 2026
…o-assertion Fix exception in `updateFormatPrewhereInfo` when only row-level filter is set
27 tasks
Desel72
pushed a commit
to Desel72/ClickHouse
that referenced
this pull request
Mar 30, 2026
…o-assertion Fix exception in `updateFormatPrewhereInfo` when only row-level filter is set
mkmkme
pushed a commit
to Altinity/ClickHouse
that referenced
this pull request
Mar 31, 2026
…o-assertion Fix exception in `updateFormatPrewhereInfo` when only row-level filter is set
zvonand
added a commit
to Altinity/ClickHouse
that referenced
this pull request
Mar 31, 2026
Antalya 26.1 Backport of ClickHouse#95476, ClickHouse#98360, ClickHouse#100361 - enable prewhere for iceberg (and fixes)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
updateFormatPrewhereInfoasserted thatprewhere_infowas non-null, but callers inStorageObjectStorageandStorageURLinvoke it when eitherprewhere_infoorrow_level_filteris set. When a row policy exists on a URL/S3 table with Parquet format (which supports PREWHERE) and the query has no PREWHERE clause, onlyrow_level_filteris populated — triggering the assertion.Additionally,
row_level_filterwas never stored into the newReadFromFormatInfo, so it would be silently lost.Fix: relax the assertion to accept either being set, and store
row_level_filterin the output.Found in: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=99537&sha=e2a32dfbb945f62e07bd3f66c206a71b584b96b1&name_0=PR&name_1=Stress%20test%20%28amd_debug%29
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Fix exception "Logical error:
prewhere_info" when querying URL/S3 Parquet tables with a row-level security policy and no PREWHERE clause.Documentation entry for user-facing changes