iframe-proxy

matanper · 2026-01-13T21:18:27Z

When querying a VIEW over a distributed table with ORDER BY + LIMIT, ClickHouse was performing a full sort on the coordinator instead of using the efficient 'merge sorted streams' optimization.

The root cause is that the Distributed table doesn't see the outer query's ORDER BY clause when reading through a VIEW, so it can't enable the optimization where shards sort their data and the coordinator merges sorted streams.

This fix pushes ORDER BY/LIMIT from the outer query into the VIEW's inner query AST in PlannerJoinTree.cpp:

Detects when the VIEW is a simple SELECT (no UNION, no JOIN, no GROUP BY, no DISTINCT, no existing ORDER BY/LIMIT)
Validates that ORDER BY contains only simple column references from the view
Clones the VIEW's inner query and adds the ORDER BY/LIMIT clauses
Sets the modified query as view_query so the Distributed table sees the ORDER BY

This enables:

Each shard to sort its data locally (or use read-in-order optimization)
The coordinator to merge pre-sorted streams efficiently
Significant performance improvement for ORDER BY + LIMIT queries

The pushdown is intentionally conservative and is skipped whenever it could change results: outer-query filtering (WHERE/PREWHERE/QUALIFY), aggregation/DISTINCT/window functions, LIMIT BY, OFFSET, WITH TIES, WITH FILL, fractional LIMIT, extremes, row policies, additional_table_filters, view/inner-query window or QUALIFY, a column type that differs between the view's declared structure and the inner query, and limit/offset/prefer_column_name_to_alias set either in the view's own SETTINGS clause or in the effective view context (e.g. through a SQL SECURITY DEFINER view's definer settings profile).

Example:

CREATE VIEW v AS SELECT * FROM distributed_table;
SELECT * FROM v ORDER BY col LIMIT 100;

Before: Full sort on coordinator (Sorting (Sorting for ORDER BY))
After: Sort on shards + merge sort on coordinator (Sorting (Merge sorted streams...))

Performance evidence

This optimization removes work on the coordinator: without it the view is an opaque subquery, so every shard streams all of its rows to the coordinator, which then performs the global ORDER BY/LIMIT. With it, each shard applies ORDER BY/LIMIT locally and the coordinator only merges pre-sorted streams (Merge sorted streams), receiving just the per-shard top-n.

Measured on test_cluster_two_shards_localhost (2 shards, 10M rows each = 20M total), SELECT id FROM v ORDER BY ts DESC LIMIT 10, max_threads = 4, median of 7 runs. The A/B was done on the same binary by toggling only the optimization via prefer_column_name_to_alias = 1 — a pure analysis-time flag that disables the pushdown without changing execution; the two outputs are byte-identical (verified by hashing the result), so the comparison isolates this change with no other commits as confounds.

On a single machine the wall time is unchanged (equal within run-to-run noise): test_cluster_two_shards_localhost runs its shards in-process (NetworkReceiveBytes = 0), so there is no transfer to save and the per-shard table scan — identical on both paths — dominates. This is exactly why the standard/cloud benchmark suites report no_change: they contain no view-over-networked-Distributed ORDER BY/LIMIT scenario. The transferred-bytes figure (measured by forcing the real network path with prefer_localhost_replica = 0, where the coordinator ingests every shard's rows) quantifies what the optimization eliminates; the benefit scales with row count × shard count and with network latency/bandwidth on real multi-node clusters. The plan-level change (coordinator full sort → Merge sorted streams) is asserted by the stateless test 04241_view_orderby_distributed_optimization.

Changelog category (leave one):

Performance Improvement

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Queries with ORDER BY and LIMIT on VIEWs over distributed tables now use the merge-sorted-streams optimization. The outer ORDER BY/LIMIT is pushed into a simple view's inner query so each shard sorts its data locally and the coordinator merges pre-sorted streams, instead of performing a full sort on the coordinator.

Closes: #92746

Note

Medium Risk
Touches query planning/AST rewriting for VIEW reads; guarded by many semantic checks but regressions are possible in edge cases around filtering/limits/order modifiers.

Overview
Enables an optimization where outer ORDER BY + LIMIT is pushed into a simple VIEW’s inner SELECT (by rewriting SelectQueryInfo::view_query) so underlying Distributed reads can produce merge-sorted streams instead of forcing a full coordinator sort.

The pushdown is intentionally conservative: it only applies to transparent views (single SELECT, no JOIN/GROUP BY/DISTINCT, no existing ORDER BY/LIMIT) and is disabled for outer-query features that can change semantics (filters, OFFSET, LIMIT BY, WITH TIES, WITH FILL, windows, row policies, additional_table_filters), with new stateless tests covering both the positive case and the main rejection cases.

^{Reviewed by Cursor Bugbot for commit 7f63adb. Bugbot is set up for automated code reviews on this repo. Configure here.}

Version info

Merged into: 26.7.1.492

…mization When querying a VIEW over a distributed table with ORDER BY + LIMIT, ClickHouse was performing a full sort on the coordinator instead of using the efficient 'merge sorted streams' optimization. The issue occurs because the VIEW's inner query has no ORDER BY clause. Without it, shards return unsorted data, forcing the coordinator to do a full sort rather than merging pre-sorted streams. This fix injects the outer query's ORDER BY into the VIEW's inner query AST before it's passed to the interpreter. The fix: 1. Detects when the outer query has ORDER BY and the table expression is a VIEW 2. Clones the VIEW's inner query AST 3. Builds ORDER BY elements from the outer query's sort columns 4. Injects the ORDER BY into the VIEW's inner query 5. Sets this modified query as view_query in SelectQueryInfo This enables: - Each shard to sort its data (or use read-in-order optimization) - The coordinator to merge sorted streams efficiently - Significant performance improvement for ORDER BY + LIMIT queries The optimization only applies to simple VIEWs without existing ORDER BY, GROUP BY, DISTINCT, LIMIT, HAVING, or WINDOW clauses. Example: CREATE VIEW v AS SELECT * FROM distributed_table; SELECT * FROM v ORDER BY col LIMIT 100; Before: Full sort on coordinator (O(N log N)) After: Sort on shards + merge sort on coordinator (more efficient)

CLAassistant · 2026-01-13T21:18:51Z

All committers have signed the CLA.

clickhouse-gh · 2026-01-13T22:42:29Z

Workflow [PR], commit [3b7bc18]

Summary: ✅

Performance Comparison: Performance dashboard

AI Review

Summary

This PR pushes outer ORDER BY / LIMIT into simple VIEW inner queries so Distributed can expose the merge-sorted-streams path instead of forcing a full coordinator sort. I did not find a remaining correctness, coverage, or PR-metadata issue on the current head: the risky semantic cases raised earlier in the review are now guarded in pushOrderByIntoView, and the new 04241_view_orderby_distributed_optimization test covers the important rejection paths for this optimization.

Final Verdict

✅ Approve

For simple views over a single table (no UNION, no JOIN), StorageView now delegates getQueryProcessingStage() to the underlying storage. This preserves the underlying storage's capabilities, such as: - Distributed tables can sort on shards with merge sort at coordinator - Other storages that support higher processing stages This fixes the bug where ORDER BY pushdown optimization was not applied to VIEWs over distributed tables, causing inefficient full sorts instead of merge sorted streams. The fix makes VIEW transparent with respect to query processing stage, which is a cleaner approach than injecting ORDER BY into the view's inner query.

The previous commit made StorageView delegate getQueryProcessingStage() to the underlying storage for ORDER BY optimization. However, this broke queries with aggregation (e.g., SELECT sum(a) FROM view) because: - The Distributed table returns stages like WithMergeableState expecting partial aggregate function states from shards - But the VIEW's inner query produces raw columns, not aggregate states - This caused: "Conversion from UInt64 to AggregateFunction(sum, UInt64) is not supported" Fix: Only delegate to underlying storage when the outer query doesn't require aggregation. We check: - query_info.need_aggregate (captures GROUP BY and aggregate functions) - query_node->isDistinct() (DISTINCT has similar merging requirements) This preserves the ORDER BY merge-sort optimization while fixing the aggregation bug. Test results: - SELECT * FROM view ORDER BY x LIMIT 10 -> Uses merge sorted streams ✓ - SELECT sum(a) FROM view -> Works correctly (returns 52) ✓ - SELECT DISTINCT a FROM view -> Falls back to FetchColumns ✓

Add NOLINT comment to empty catch block in getQueryProcessingStage. The empty catch is intentional - it allows fallback to default behavior when table lookup fails during query processing stage determination. This follows the established pattern used throughout the codebase for intentional exception swallowing.

…r-distributed-table-order-by-pushdown

…er-by-pushdown

- Return FetchColumns early when query_tree is not available (analyzer disabled) - Use query tree for outer query analysis (aggregation, DISTINCT checks) - Simplify control flow with early returns instead of nested conditionals - Keep metadata-based approach for finding underlying table in view definition The optimization delegates getQueryProcessingStage to the underlying storage for simple views (single SELECT, no UNION, no JOIN), enabling ORDER BY optimizations like merge sorted streams for distributed tables.

…er-by-pushdown

When querying a VIEW over a Distributed table with ORDER BY, the optimization that merges pre-sorted streams from shards was not applied. This is because the ORDER BY clause wasn't visible to the underlying distributed table. This change pushes ORDER BY (and LIMIT) from the outer query into the VIEW's inner query AST, enabling the distributed table to see the sort requirement and apply the merge-sorted-streams optimization instead of doing a full sort after receiving all data. Also fix the test to use EXPLAIN instead of EXPLAIN PIPELINE, since "Merge sorted streams" is a step description that appears in query plan output, not pipeline output.

When querying a VIEW over a Distributed table with ORDER BY, the optimization that merges pre-sorted streams from shards was not applied. This is because the ORDER BY clause wasn't visible to the underlying distributed table. This change pushes ORDER BY (and LIMIT) from the outer query into the VIEW's inner query AST in PlannerJoinTree, enabling the distributed table to see the sort requirement and apply the merge-sorted-streams optimization instead of doing a full sort after receiving all data. Simplified approach: Instead of complex stage delegation in StorageView, we just push ORDER BY into view_query which is used by StorageView::read(). The underlying Distributed table then naturally sees the ORDER BY. Also fix the test to use EXPLAIN instead of EXPLAIN PIPELINE, since "Merge sorted streams" is a step description that appears in query plan output, not pipeline output.

…b.com:matanper/ClickHouse into view-over-distributed-table-order-by-pushdown

…er-by-pushdown

The setup dropped the `definer_*_04241` users before dropping any leftover `test_view_definer_*_04241` views from a previous interrupted run. Since the views depend on the users, `DROP USER IF EXISTS` would throw `HAVE_DEPENDENT_OBJECTS` while a leftover view still exists. Drop the dependent views before the users (both in the initial cleanup block and right before the `DROP USER` statements). Addresses review feedback on ClickHouse#94102 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

alexey-milovidov · 2026-06-20T08:07:10Z

Continued work on this PR (pushed `736e525ce64..e0449a6`):

Merged latest `master` (the PR was conflicting). The only conflict was in `src/Planner/PlannerJoinTree.cpp`: this PR and master each added independent helper functions at the same spot (`pushOrderByIntoView`/`containsWindowFunction` here, `parallelReplicasEnabledForStorage`/`allowParallelReplicasForJoinTree` on master). Kept both; the net diff of the PR is unchanged (the same 3 files).

Addressed both remaining review findings (AI Review “⚠️ Request changes”):

Guard the pushdown under `exact_rows_before_limit` (cf6e276). pushOrderByIntoView now returns early when exact_rows_before_limit is set, the same way it already skips extremes. Otherwise the pushed inner LIMIT becomes a child LimitTransform and initRowsBeforeLimit (which ignores child limits once it finds the outer limit) would report only the per-shard top-N instead of the full pre-LIMIT count.
Make the definer-view test rerunnable (e0449a6). 04241 now drops the test_view_definer_*_04241 views before the DROP USER statements, so a partially-failed previous run no longer trips HAVE_DEPENDENT_OBJECTS.

Added a regression test (8250201): 04241 now asserts rows_before_limit_at_least reports the full pre-LIMIT count (200 = 100 rows × 2 shards) for a view-over-Distributed ORDER BY ... LIMIT with SETTINGS exact_rows_before_limit = 1, and that the pushdown is disabled in that case.

Local validation (against a 26.6 server with test_cluster_two_shards_localhost): ran the full 04241 test; the only lines that differ from the reference are the positive pushdown assertions that require this PR’s build (master has no view pushdown), while every negative case and the new exact_rows_before_limit block match exactly. The C++ change is a one-line guard mirroring the existing extremes check and compiles cleanly (-fsyntax-only). Also reproduced the original DROP USER → HAVE_DEPENDENT_OBJECTS failure and confirmed the cleanup-order fix resolves it.

The only CI failure on the prior commit was Stress test (arm_tsan) → Logical error: Unexpected exception in refresh scheduling (STID: 2508-2e88) in RefreshTask.cpp, which is unrelated to this planner/AST change and tracked as a known master-wide flake in #106737.

# Conflicts: # src/Planner/PlannerJoinTree.cpp

`arrayJoin` used as a projection function changes row cardinality after the source read: it expands each input row into one row per array element and drops rows whose array is empty. Unlike an `ARRAY JOIN` clause (which the analyzer lowers to a separate table expression, so the outer query is no longer a single-table read and never reaches the optimization), `arrayJoin` in the select list keeps `is_single_table_expression` true and slips past the existing guards in `pushOrderByIntoView`. Pushing `ORDER BY`/`LIMIT` into the view then truncates source rows before the expansion runs, so for a query like `SELECT arrayJoin(arr) FROM v ORDER BY ts DESC LIMIT 10` where the top ordered rows have empty arrays, the rewritten query would return too few rows instead of continuing to lower ordered rows to fill the `LIMIT`. Reject the optimization when the outer projection contains `arrayJoin`, mirroring the existing guard in `mainQueryNodeBlockSizeByLimit`, and add a focused regression to `04241_view_orderby_distributed_optimization`. Addresses AI Review finding on PlannerJoinTree.cpp:940. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

alexey-milovidov · 2026-06-25T07:28:54Z

Continued work on this PR (pushed eed9bb9d436..68bbd422537):

Addressed the AI Review ⚠️ Request changes finding (arrayJoin in the outer projection). arrayJoin used as a projection function changes row cardinality after the source read (it expands each row per array element and drops empty-array rows). Unlike an ARRAY JOIN clause — which the analyzer lowers to a separate table expression, so the query is no longer single-table and never reaches the optimization — arrayJoin in the select list kept is_single_table_expression true and slipped past the existing guards in pushOrderByIntoView. Pushing ORDER BY/LIMIT into the view could then truncate source rows before the expansion, returning too few rows when the top ordered rows have empty arrays.

Fix: reject the optimization when hasFunctionNode(outer->getProjectionNode(), "arrayJoin"), mirroring the existing guard in mainQueryNodeBlockSizeByLimit.
Regression test added to 04241_view_orderby_distributed_optimization: a view over Distributed exposes an array column where the highest-ts rows have empty arrays; SELECT arrayJoin(arr) FROM v ORDER BY ts DESC LIMIT 10 must not use merge-sorted-streams and must still return 10 rows from lower-ts rows (verified — a buggy pushdown would return 0).

The C++ change passes -fsyntax-only; the new test queries were validated against a master server (they behave identically to master once the guard disables the pushdown).

Note on the prior CI report for eed9bb9d436: the 101 red entries were all jobs that never ran (ERROR, no start time) — a transient CI-orchestration error that failed to schedule the release/cross-compile build wave and everything downstream of it. Every job that actually executed passed (all debug/sanitizer/binary/arm builds 0/4, Fast test 9213 passed, flaky checks). This push re-triggers CI.

…ed-table-order-by-pushdown

alexey-milovidov · 2026-06-30T04:55:54Z

Merged the latest master (pushed 68bbd422537..c4f8f90b248) and re-triggered CI.

The merge was clean — no conflicts. The net PR diff is unchanged (still only src/Planner/PlannerJoinTree.cpp plus the 04241_view_orderby_distributed_optimization test). The merged PlannerJoinTree.cpp passes -fsyntax-only against current master.
The only non-green public check was Stress test (arm_tsan), which failed with the chronic Logical error: 'Unexpected exception in refresh scheduling' in RefreshTask::doScheduling (STID 2508-3e7b, #106737) followed by Cannot start clickhouse-server. This is unrelated to this planner-only change — already confirmed above by @groeneai (covered by fix PR Fix null deref in RefreshTask::doScheduling under parallel shutdown #105588). The fresh-master re-run should clear it.
CH Inc sync failed on the now-superseded head 68bbd422; the fresh-master merge re-triggers it.

No unresolved review threads; the AI Review verdict is ✅ Approve.

The merge to `master` made `explain_query_plan_default = 'pretty'` the default, so `EXPLAIN PLAN` now decorates each step with tree-drawing characters (`└──`/`├──`/`│`). The step-extraction query in `04241_view_orderby_distributed_optimization` stripped only leading whitespace, so the extracted step became `└──Sorting (Merge sorted streams after aggregation stage for ORDER BY)` instead of the reference `Sorting (Merge sorted streams ...)`. Strip leading whitespace and the tree-drawing prefix characters so the extracted step text is stable regardless of the step's depth and position in the plan tree (the surrounding plan shape can vary across builds, which is why the test matches only this one line). CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=94102&sha=c4f8f90b2482e0d92bfb192abca00c57171466e4&name_0=PR&name_1=Fast%20test Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ed-table-order-by-pushdown

alexey-milovidov · 2026-06-30T07:15:50Z

Pushed c4f8f90b248..c59fa22d793 and re-triggered CI.

Fixed the sole CI failure (Fast test → 04241_view_orderby_distributed_optimization). The clean master merge in c4f8f90b248 picked up the change that made explain_query_plan_default = 'pretty' the default, so EXPLAIN PLAN now decorates each step with tree-drawing characters (└──/├──/│). The step-extraction query stripped only leading whitespace (^\\s+), so the extracted step became └──Sorting (Merge sorted streams after aggregation stage for ORDER BY) instead of the reference Sorting (Merge sorted streams ...):

-Sorting (Merge sorted streams after aggregation stage for ORDER BY)
+└──Sorting (Merge sorted streams after aggregation stage for ORDER BY)

The regex now strips leading whitespace and the pretty tree-drawing prefix characters (^[\\s│├└─]+), so the extracted step text is stable regardless of the step's depth/position in the plan tree (the surrounding plan shape can vary across builds, which is why the test matches only this one line). Verified with clickhouse-local that both └──Sorting (…) and a nested │ └──Sorting (…) reduce to the expected Sorting (Merge sorted streams after aggregation stage for ORDER BY). The reference file is unchanged. CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=94102&sha=c4f8f90b2482e0d92bfb192abca00c57171466e4&name_0=PR&name_1=Fast%20test

Also merged the latest master (the branch was 7 commits behind). The merge was clean — master's only touch of src/Planner/PlannerJoinTree.cpp was content-neutral relative to the merge-base, and the new commits ("remove strange trash", test renumbering, upgrade-check filter) do not affect this PR or the EXPLAIN format. Net PR diff is unchanged: only src/Planner/PlannerJoinTree.cpp and the 04241 test/reference.

No unresolved review threads; the AI Review verdict is ✅ Approve. The remaining CHANGES_REQUESTED decision is from older, un-dismissed reviews (novikd, and the empty . review) whose threads are all resolved — clearing those is a maintainer action.

alexey-milovidov · 2026-06-30T09:52:37Z

The only non-green check on the current head c59fa22d793 is Stress test (arm_debug), which failed with the chronic Logical error: 'Not-ready Set is passed as the second argument for function 'notNullIn'' (STID 0250-4e52, #102803). This is unrelated to this planner-only change:

The crashing query (query_id a91d2cbd…, comment 01231_operator_null_in_2.sql) is SELECT count() = 65537 FROM null_in_subquery WHERE i NOT IN (SELECT i FROM null_in_subquery WHERE dt = '1970-01-01 00:00:00.000000000') LIMIT 315. null_in_subquery is a plain (non-distributed) MergeTree table and there is no VIEW involved, so pushOrderByIntoView cannot apply — it also bails out on any outer WHERE/PREWHERE. The fatal frame is in FilterTransform, i.e. the NOT IN/notNullIn set, not in the planner.
It is a broad master-wide flake (labels testing, fuzz): STID 0250-4e52 has fired 15× on master and across 68 distinct PRs in the last 90 days (last seen today). Root-cause/fix work is already in flight: Fix "Not-ready Set" exception by making in-place set build non-destructive #107924 and the drafts Restore subquery plans after failed in-place set build #102308 / Fix "Not-ready Set" exception when buildOrderedSetInplace fails #102192.

CI for the fresh-master head is still running and the rest is green; no unresolved review threads, AI Review verdict ✅ Approve.

@groeneai, please confirm/track this failure (report) and link the in-progress fix here if one of the above PRs covers it.

alexey-milovidov · 2026-06-30T10:40:05Z

A second red appeared on the current head c59fa22d793 after my previous comment: Stress test (amd_asan_ubsan) → Hung check failed, possible deadlock found (report). This job completed at 09:56Z, just after my 09:52Z note that only covered arm_debug, so flagging it explicitly.

It is unrelated to this planner-only change (the PR touches only src/Planner/PlannerJoinTree.cpp plus the 04241 test/reference):

The hung stack is entirely in query-tree analysis of a fuzzed SELECT — DB::QueryTreePassManager::run → buildQueryTreeAndRunPasses → InterpreterSelectQueryAnalyzer → executeQueryImpl. There are zero PlannerJoinTree / pushOrderByIntoView frames; the pushdown runs much later (planning) and only for a transparent VIEW over Distributed, which the fuzzer corpus here does not exercise.
This is the chronic master-wide Hung-check flake, #107941 (OPEN). CIDB last 30 days: 1591 failures across 593 distinct PRs and 428 hits directly on master, last seen today. It is not specific to this PR.

So both non-green checks on c59fa22d793 are pre-existing, tracked flakes:

Stress test (arm_debug) → Not-ready Set ... 'notNullIn' (STID 0250-4e52, #102803) — documented above.
Stress test (amd_asan_ubsan) → Hung check (#107941) — this one.

No unresolved review threads; AI Review verdict ✅ Approve. The rest of CI on the fresh-master head is green or still running.

@groeneai, please confirm/track the Hung-check sighting and link the in-flight fix if one exists.

alexey-milovidov · 2026-07-03T01:45:17Z

Merged the latest master (pushed c59fa22d793..17f3834a507) and re-triggered CI.

The merge was clean — no conflicts (unlike the earlier master merges that collided in src/Planner/PlannerJoinTree.cpp). The net PR diff is unchanged: still only src/Planner/PlannerJoinTree.cpp plus the 04241_view_orderby_distributed_optimization test/reference.
No API drift from the ~3 days of new master: the merged PlannerJoinTree.cpp passes -fsyntax-only against current master headers.
The two non-green checks on the prior head c59fa22d793 were both pre-existing, tracked flakes, unrelated to this planner-only change (as documented above):
1. Stress test (arm_debug) → Not-ready Set ... 'notNullIn' (STID 0250-4e52, #102803).
2. Stress test (amd_asan_ubsan) → Hung check (#107941).
  The fresh-master re-run should clear them.

No unresolved review threads; the AI Review verdict is ✅ Approve. The remaining CHANGES_REQUESTED decision is from older, un-dismissed reviews (novikd, and the empty . review) whose threads are all resolved — clearing those is a maintainer action.

Address review: `pushOrderByIntoView` rebuilt the pushed-down inner query from the view's stored definition (`metadata->getSelectQuery().inner_query`), which does not carry the `SAMPLE`/`FINAL` modifiers applied to the view in the outer query. Pushing the outer `ORDER BY`/`LIMIT` into that inner query would order and truncate the full, unsampled, non-final row set below the point where the modifier applies, so a query such as `SELECT id FROM v SAMPLE 0.1 ORDER BY ts DESC LIMIT 10` could return the top-N of the whole view instead of the top-N of the sampled subset. Skip the optimization whenever the view table expression carries a `SAMPLE` or `FINAL` modifier, mirroring the existing modifier guard used for the trivial-count optimization in the same file. Added `FINAL`/`SAMPLE` rejection cases to `04241_view_orderby_distributed_optimization`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

alexey-milovidov · 2026-07-03T06:15:41Z

Addressed the AI review blocker and pushed 17f3834a507..3b7bc18b0b6.

Fix — skip the pushdown when the view carries SAMPLE/FINAL modifiers. pushOrderByIntoView rebuilt the pushed-down inner query from the view's stored definition (metadata->getSelectQuery().inner_query), which does not carry the SAMPLE/FINAL modifiers applied to the view in the outer query. Pushing the outer ORDER BY/LIMIT into that inner query would order/truncate the full, unsampled, non-final row set below the point where the modifier applies, so e.g. SELECT id FROM v SAMPLE 0.1 ORDER BY ts DESC LIMIT 10 could return the top-N of the whole view instead of the sampled subset. Added a guard that skips the optimization whenever the view table expression carries a SAMPLE/FINAL modifier — mirroring the existing modifier guard used for the trivial-count optimization in the same file — plus FINAL/SAMPLE rejection cases in 04241_view_orderby_distributed_optimization. -fsyntax-only on the change passes against current master headers.

CI on the previous head 17f3834a507 (the master re-merge): all three reds were pre-existing, tracked flakes, unrelated to this planner-only change:

Stateless tests (amd_asan_ubsan, db disk, distributed plan …) — Server died + AddressSanitizer in MergeTask::ExecuteAndFinalizeHorizontalPart::executeImpl (background merge). Chronic master-wide ASan-in-merge flake: 6 hits on master + 33 PRs in 60 days, last seen 2026-07-03. No view/planner involvement.
Stress test (arm_release) — Logical error: Block structure mismatch in JoinStep in JoinStep::updatePipeline (fuzzer). The byte-identical crash ([__table3.number, __table3.number]) fired on unrelated PRs Add zero-argument year() returning the current year #109097 and Add allow_lossy_numeric_supertype setting for numeric Variant common types #107236 before this one; tracked by the open flake issue Logical error: Block structure mismatch in A stream: different number of columns: (STID: 0993-308a) #109215. Stack is pure InterpreterSelectQueryAnalyzer::buildQueryPipeline — no PlannerJoinTree/pushOrderByIntoView frame.

clickhouse-gh · 2026-07-03T09:54:57Z

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	85.50%	85.50%	+0.00%
Functions	92.70%	92.60%	-0.10%
Branches	77.70%	77.70%	+0.00%

Changed lines: Changed C/C++ lines covered: 111/118 (94.07%) · Uncovered code

Full report · Diff report

Resolved.

groeneai · 2026-07-03T15:20:44Z

alexey-milovidov added the can be tested Allows running workflows for external contributors label Jan 13, 2026

clickhouse-gh Bot added the pr-performance Pull request with some performance improvements label Jan 13, 2026

matanper force-pushed the view-over-distributed-table-order-by-pushdown branch from 40343e1 to fdbc3ba Compare January 13, 2026 23:47

matanper added 4 commits January 14, 2026 10:40

Merge branch 'master' of github.com:matanper/ClickHouse into view-ove…

7c87de7

…r-distributed-table-order-by-pushdown

Merge branch 'ClickHouse:master' into view-over-distributed-table-ord…

1a1b70a

…er-by-pushdown

novikd self-assigned this Jan 19, 2026

novikd previously requested changes Jan 20, 2026

View reviewed changes

Comment thread src/Storages/StorageView.cpp Outdated

Comment thread src/Storages/StorageView.cpp Outdated

Comment thread src/Storages/StorageView.cpp Outdated

Comment thread tests/queries/0_stateless/03788_view_orderby_distributed_optimization.sql Outdated

matanper added 6 commits January 21, 2026 01:21

Merge branch 'ClickHouse:master' into view-over-distributed-table-ord…

006fb6f

…er-by-pushdown

Merge branch 'ClickHouse:master' into view-over-distributed-table-ord…

d2af479

…er-by-pushdown

Merge branch 'view-over-distributed-table-order-by-pushdown' of githu…

30a9cf9

…b.com:matanper/ClickHouse into view-over-distributed-table-order-by-pushdown

matanper force-pushed the view-over-distributed-table-order-by-pushdown branch 5 times, most recently from e926237 to 3a3d6db Compare January 24, 2026 11:58

matanper requested a review from novikd January 24, 2026 11:58

matanper force-pushed the view-over-distributed-table-order-by-pushdown branch from 3a3d6db to d71d487 Compare January 24, 2026 12:22

Merge branch 'ClickHouse:master' into view-over-distributed-table-ord…

9b98bb2

…er-by-pushdown

matanper force-pushed the view-over-distributed-table-order-by-pushdown branch from d71d487 to 513fb91 Compare January 24, 2026 12:36

alexey-milovidov unassigned novikd Mar 25, 2026

alexey-milovidov reviewed Mar 25, 2026

View reviewed changes

Comment thread src/Planner/PlannerJoinTree.cpp Outdated

Merge remote-tracking branch 'origin/master' into HEAD

eed9bb9

# Conflicts: # src/Planner/PlannerJoinTree.cpp

clickhouse-gh Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread src/Planner/PlannerJoinTree.cpp

Merge remote-tracking branch 'origin/master' into view-over-distribut…

c4f8f90

…ed-table-order-by-pushdown

alexey-milovidov and others added 2 commits June 30, 2026 07:14

Merge remote-tracking branch 'origin/master' into view-over-distribut…

c59fa22

…ed-table-order-by-pushdown

Merge remote-tracking branch 'origin/master' into pr-94102-view-orderby

17f3834

clickhouse-gh Bot reviewed Jul 3, 2026

View reviewed changes

Comment thread src/Planner/PlannerJoinTree.cpp

alexey-milovidov added can be tested Allows running workflows for external contributors and removed can be tested Allows running workflows for external contributors labels Jul 3, 2026

alexey-milovidov self-requested a review July 3, 2026 14:36

alexey-milovidov approved these changes Jul 3, 2026

View reviewed changes

alexey-milovidov added this pull request to the merge queue Jul 3, 2026

Merged via the queue into ClickHouse:master with commit f62a9d3 Jul 3, 2026
174 checks passed

robot-ch-test-poll3 mentioned this pull request Jul 3, 2026

Query loses “merge sorted streams” optimization when querying through a VIEW over Distributed table #92746

Closed

robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 3, 2026

Path	Rows the coordinator must ingest + globally sort	Wall time, single machine
Without pushdown	all 20M `(id, ts)` rows ≈ 228.9 MiB transferred over the network path	~66 ms
With pushdown	per-shard top-10 (≈20 rows total)	~67 ms

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

matanper commented Jan 13, 2026 • edited by robot-ch-test-poll3 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance evidence

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Version info

Uh oh!

CLAassistant commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clickhouse-gh Bot commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Review

Summary

Final Verdict

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexey-milovidov commented Jun 20, 2026

Uh oh!

Uh oh!

alexey-milovidov commented Jun 25, 2026

Uh oh!

alexey-milovidov commented Jun 30, 2026

Uh oh!

alexey-milovidov commented Jun 30, 2026

Uh oh!

alexey-milovidov commented Jun 30, 2026

Uh oh!

alexey-milovidov commented Jun 30, 2026

Uh oh!

alexey-milovidov commented Jul 3, 2026

Uh oh!

Uh oh!

alexey-milovidov commented Jul 3, 2026

Uh oh!

clickhouse-gh Bot commented Jul 3, 2026

LLVM Coverage Report

Uh oh!

Uh oh!

groeneai commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

matanper commented Jan 13, 2026 •

edited by robot-ch-test-poll3

Loading

CLAassistant commented Jan 13, 2026 •

edited

Loading

clickhouse-gh Bot commented Jan 13, 2026 •

edited

Loading