Support PREWHERE and trivial count for Memory tables - #116248
Support PREWHERE and trivial count for Memory tables#116248alexey-milovidov wants to merge 12 commits into
Conversation
PREWHERE (and the pushed-down row-level security filter) is applied inside MemorySource: only the columns of the conditions are read at first, and the remaining columns are read only for the blocks where some rows pass and only for the passing rows. For a table with SETTINGS compress = true a selective condition skips decompression of all other columns for the blocks it eliminates. StorageMemory::getColumnSizes reports real per-column in-memory sizes (compressed sizes when compress = true), which both enables the automatic WHERE -> PREWHERE move in the query plan optimization and lets it order conditions by the actual cost of reading their columns. SELECT count() FROM table is served from metadata (totalRows is exact, maintained under the write mutex). Motivated by benchmarking a compressed Memory table: ClickHouse/ClickBench#1590 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s and docs A materialized CTE and a GLOBAL subquery temporary table are filled during query execution, after the planner would have observed totalRows (as zero), so the trivial count optimization must not apply to them. Also update the in-code Memory engine documentation and add functional and performance tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ents Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Workflow [PR], commit [9614350] Summary: ❌
AI ReviewSummaryThis PR adds in-source Findings❌ Blockers
Missing context / blind spots
Final Verdict
|
clickhouse-gh
Bot
added
the
pr-performance
|
📊 Cloud Performance Report ✅ AI verdict: This PR adds PREWHERE and row-level-filter support plus a trivial count() optimization to the Memory table engine and its reading source. All flagged clickbench queries (Q18, Q28, Q30, Q31, Q33) run against MergeTree tables and never exercise the Memory read path, so none of the +5% to +19% per-query deltas can be caused by this change. Each envelope-suppressed row sits inside master's current variance band and Q28 shows mixed test signals, so these are run-to-run variance rather than PR effects. No regression or improvement is attributable to the diff, and no action is warranted. clickbenchFlagged queries (3 of 43)
Change = percent below ×2; the ratio of medians (×N faster/slower) beyond, where percent understates the scale. q-value = BH-FDR adjusted p; smaller is stronger evidence. MIRAI flags a query when q < fdr_q (default 0.10) — the value the verdict is based on. tpch_adapted_1_official🟢 No significant changes Debug info
|
Virtual columns (e.g. `_table`) are materialized outside the reading source, so the in-source filter cannot read them: `SELECT * FROM t PREWHERE _table = 't'` failed with `NOT_FOUND_COLUMN_IN_BLOCK`. Declare `supportedPrewhereColumns`, so both the analyzer and the plan-level WHERE -> PREWHERE optimization reject such conditions with `ILLEGAL_PREWHERE`, as asserted by `03094_virtual_column_table_name`. https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=116248&sha=ebb9ae35257ee1588af487bfadfbb6002ac9c7d6&name_0=PR&name_1=Fast%20test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ake) Generated by running the tests; the values match manual computation and the outputs observed in the CI report https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=116248&sha=ebb9ae35257ee1588af487bfadfbb6002ac9c7d6&name_0=PR&name_1=Fast%20test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- `03610_disjunctions_pushdown_optimization` already pins `optimize_move_to_prewhere` and `query_plan_optimize_prewhere` to 1, so the pushed-down disjunctions over `Memory` tables now become PREWHERE; update the expected plan accordingly. - `03777_join_precalculate_keys` and `03707_analyzer_convert_outer_any_to_inner` assert on join plans, so pin `optimize_move_to_prewhere = 0` to keep the asserted plans independent of the (harness-randomized) PREWHERE move. - `03562_short_circuit_for_and_or` asserts on `read_rows` of count subqueries to prove short circuit; pin `optimize_trivial_count_query = 0`, because serving the count of a `Memory` table from metadata would defeat that signal. https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=116248&sha=ebb9ae35257ee1588af487bfadfbb6002ac9c7d6&name_0=PR&name_1=Fast%20test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
Build profile diff (arm_release)Comparing ✅ No significant changes. Binary sizes
The official master build is compiled with Object file sizes7 object files changed (+69.56 KiB total), 0 added.
716 more object files are built by the master warmup baseline only (it builds every object-file target, a pull request build only Compile time of recompiled translation units13 translation units recompiled, 75 s compile time in total, 13 of them have a recent master baseline. |
…ocks `SELECT count()` on a `Memory` table is served from the row counter, while ordinary reads use the set of blocks captured in `getStorageSnapshot`. The counters were separate atomics updated around `data.set`, so a concurrent reader could observe a row count that corresponded to no state the table ever had. Move the row and byte counters into the `MultiVersion` object that holds the blocks, as the pre-existing `TODO` in `getStorageSnapshot` suggested. They are now published atomically together with the blocks they describe, `totalRows` is the exact row count of a committed state, and `SnapshotData::rows` is exact rather than approximate. Also restrict `supportedPrewhereColumns` to the stored columns without a `DEFAULT` expression (the same restriction as `StorageFile`): such a column is absent from the blocks written before `ALTER TABLE ... ADD COLUMN`, and the in-source filter reads it as the default value of its type instead of evaluating the expression. `ALIAS` and `EPHEMERAL` columns are excluded as well, because they are never stored. Add `05052_memory_prewhere_added_column` and `05053_memory_trivial_count_concurrent`.
…er test `ReadFromMemoryStorageStep` evaluates the row-level security filter and `PREWHERE` inside `MemorySource`, so a condition such as `PREWHERE k IN (SELECT ...)` carries a `FutureSet` that has to be ready by the time the source runs. The pipeline-level `CreatingSetsStep` normally fills it in, but `DelayedPortsProcessor` can be short-circuited by a downstream processor that closes its inputs early, which is why `ReadFromMergeTree` builds those sets in place for its storage-level `PREWHERE`. Do the same in `makeSourceFilter`, excluding the sets of `GLOBAL IN`, to which `ReadFromRemote` still has to attach an external table. Added `05057_memory_prewhere_in_subquery` covering explicit and optimizer-moved `PREWHERE ... IN (SELECT ...)` and a row policy with `IN`. `05052_memory_prewhere_added_column` failed with the old analyzer: it substitutes an `ALIAS` column expression into `PREWHERE` before the storage sees it, so `PREWHERE a = 4` is not rejected there. Pin `enable_analyzer` for that assertion. Renumbered the two tests that collided with master. https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=116248&sha=de59dc98eb76385347efe69e31ca5bf9b02bdf47&name_0=PR&name_1=Stateless%20tests%20%28amd_llvm_coverage%2C%20old%20analyzer%2C%20s3%20storage%2C%20DBReplicated%2C%20parallel%2C%202%2F3%29 #116248
| return Chunk(std::move(columns), num_rows); | ||
| if (filter) | ||
| { | ||
| if (auto chunk = generateFiltered(src)) |
There was a problem hiding this comment.
ISource still auto-reports progress from the returned Chunk, so after this change it sees only the rows that survive PREWHERE, and this continue path reports nothing at all for a block that the filter eliminates completely. That regresses read_rows / SelectedRows accounting and weakens max_rows_to_read / read-quota enforcement for selective Memory scans; if every block is filtered out, PipelineExecutor never calls ReadProgressCallback::onProgress() for this source. Could we switch the filtered MemorySource path to manual progress() calls and account src.rows() for every scanned block, even when no rows survive?
| if (query_context->hasQueryContext() && query_context->getQueryContext()->getPinnedStorageSnapshot(getStorageID().uuid)) | ||
| return false; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
Returning true here also turns on trivial count through wrappers such as StorageMerge, which decides at the parent level and then sums each child table->totalRows(query_context). Child row policies are only attached later when StorageMerge builds the child read plans (RowPolicyData in createChildrenPlans), so SELECT count() on a Merge table over a Memory child with a row policy can now return the unfiltered row count instead of the policy-filtered one. Direct Memory row-policy tests cover only the base table path. I think we either need to make the wrapper path row-policy-aware before advertising trivial count from Memory, or keep this returning false until that exists.
1 participant
Related: ClickHouse/ClickBench#1590
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Support
PREWHERE(including the automatic move ofWHEREconditions byoptimize_move_to_prewhere) forMemorytables: only the columns of the conditions are read at first, and the remaining columns are read only for the blocks where some rows pass, and only for the passing rows. This is especially beneficial for tables withSETTINGS compress = true, because for a selective condition most columns are never decompressed. Additionally,SELECT count() FROM tableon aMemorytable is now served from metadata, andsystem.columnsshows real per-column sizes forMemorytables.The motivation is benchmarking a compressed in-memory table on ClickBench (ClickHouse/ClickBench#1590), where the last seven queries (
CounterID = 62) lose the primary index ofMergeTreeand previously had to decompress every referenced column of every block.Implementation:
StorageMemory::supportsPrewhereis now true.MemorySourceapplies the pushed-down row-level security filter and PREWHERE inside the reading source: it materializes only the filter-input columns, executes the filter steps, skips a block entirely when no row passes, and reads the remaining columns only for the surviving rows (IColumn::filterwith the combined mask). The block layout is kept in exact correspondence with the output header, whichSourceStepWithFilter::applyPrewhereActionsbuilds by running the same actions on the sample block.StorageMemory::getColumnSizesreports real per-column in-memory sizes (compressed sizes whencompress = true). This is what enables the plan-levelWHERE->PREWHEREoptimization (it declines on storages with no column sizes) and letsMergeTreeWhereOptimizerorder conditions by the actual cost of reading their columns.StorageMemory::supportsTrivialCountOptimizationis now true, guarded against tables that are filled during query execution (materialized CTEs,GLOBALsubquery temporary tables) and against pinned snapshots (atomicCREATE MATERIALIZED VIEW ... POPULATE), wheretotalRowsmust not be observed at planning time.Benchmark (ClickBench queries, 10M-row
hitssubset in aMemorytable withcompress = true, 96-core aarch64, hot runs, new binary with the optimizations toggled off/on viaoptimize_move_to_prewhere/optimize_trivial_count_query):SELECT COUNT(*)SELECT * ... URL LIKE '%google%' ORDER BY ... LIMIT 10WHERE CounterID = 62 AND EventDate ...SELECT *point lookup byWatchIDThe remaining queries are unchanged within noise. The effect grows with table size and filter selectivity: on the full 100M-row dataset the eliminated blocks dominate.
Workflow [PR]
Sync PR [sync-upstream/pr/116248]