You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
SQL top-k by COUNT(*): sketch candidates are misplaced (per-host CMS/UnivMon, heap over exact counts); no CMS+heap over raw rows #886
Q1 Cms and Q1 UnivMon might be incorrect. They keep one sketch per host, and each sketch only ever sees its own host's rows before being asked for that host's count. They are also rejected as executor_cannot_compile (unsupported weighted frequency family; summary update type is unsupported by its family).
Q1 CMS+heap also seems wrong. It builds the CMS+heap over the already-exact (host, count) rows, so the top 5 is already known before the sketch runs. Its weights are the count column, which is why it is rejected with count_min_weights_not_proven_non_negative, even though COUNT(*) is never negative. CountSketch+heap has the same shape and fails with executor_cannot_compile: summary update column missing or ambiguous.
The expected top-k sketch plan is missing. No candidate builds one CMS+heap (or CountSketch+heap) over the raw rows keyed by host and reads its top 5.
Confirmed a specific candidate-generation gate in the current planner (base main 7680b6bc). The original SQL COUNT-ranked TopK inventory has fused=None, even though the keyed-additive realization already knows how to build a global heap over raw rows with host as the item and constant weight 1.
per_computation_candidates.rs::whole_expression_input rejects closed-schema inputs when !reads_only_time_series(realized.child). Its comment explicitly keeps SQL heaps over the per-key result. This excludes SQL raw-row fusion. The new regression asserting a fused SQL heap fails before removing that SQL-only exclusion and passes afterward. The resulting candidates use raw input and unit weights, and compile through the executor when accuracy enforcement is disabled.
This is not yet a complete fix: with the issue's epsilon/delta target enforced, the newly exposed fused candidates are still rejected as NoAccuracyGuarantee. That remaining accuracy propagation/contract needs to be addressed before claiming they are selectable. The test distinguishes candidate existence/compilation from accuracy acceptance.
Also, a CMS per host is not automatically a wrong-answer plan merely because each sketch sees one host: it can be a wasteful representation of a count. The original family/update compilation problems and the missing raw-row fusion are separate concerns; they should not be conflated.
All changes/regressions are local and uncommitted. No PR/commit has been published; keep the issue open.
Implemented in PR #943. SQL count-ranked CMS/CountSketch heaps now fuse over raw rows keyed by the grouping columns with unit weights, absorbing the inner count. Both heaps compile and execute. Unsupported per-group SQL count-sketch realizations are explicitly pruned, with exact counters retained. Tests cover integer keys, a key named value, and partitioned TopK. Frequency bounds still do not certify TopK membership, so the accuracy gate correctly remains in force.
Final local validation: 690 focused tests passed, plus workspace format and all-target/all-feature Clippy checks.
Activity
milindsrivastava1997 commentedon Oct 22, 2026
Overlap check (still reproduces on
4535c957):ORDER BY c DESC LIMIT k) does not bind #619, item 2: the heap candidates for this same query shape. In SQL top-k ordered by an alias (ORDER BY c DESC LIMIT k) does not bind #619 they failed to compose; they now compose, but stacked on the exact counts instead of over the raw rows.Related: #657 (the top-k rewrite's output shape), #773 and #832 (the same rejection and the same missing fused heap, fixed for PromQL).
zzylol commentedon Oct 23, 2026
Deferred until the memo search (#879, #888) lands. This sharing/reuse rule will be added as a memo transformation rather than to the legacy search.
zzylol commentedon Oct 27, 2026
Confirmed a specific candidate-generation gate in the current planner (base main
7680b6bc). The original SQL COUNT-ranked TopK inventory hasfused=None, even though the keyed-additive realization already knows how to build a global heap over raw rows with host as the item and constant weight 1.per_computation_candidates.rs::whole_expression_inputrejects closed-schema inputs when!reads_only_time_series(realized.child). Its comment explicitly keeps SQL heaps over the per-key result. This excludes SQL raw-row fusion. The new regression asserting a fused SQL heap fails before removing that SQL-only exclusion and passes afterward. The resulting candidates use raw input and unit weights, and compile through the executor when accuracy enforcement is disabled.This is not yet a complete fix: with the issue's epsilon/delta target enforced, the newly exposed fused candidates are still rejected as NoAccuracyGuarantee. That remaining accuracy propagation/contract needs to be addressed before claiming they are selectable. The test distinguishes candidate existence/compilation from accuracy acceptance.
Also, a CMS per host is not automatically a wrong-answer plan merely because each sketch sees one host: it can be a wasteful representation of a count. The original family/update compilation problems and the missing raw-row fusion are separate concerns; they should not be conflated.
All changes/regressions are local and uncommitted. No PR/commit has been published; keep the issue open.
zzylol commentedon Oct 27, 2026
Implemented in PR #943. SQL count-ranked CMS/CountSketch heaps now fuse over raw rows keyed by the grouping columns with unit weights, absorbing the inner count. Both heaps compile and execute. Unsupported per-group SQL count-sketch realizations are explicitly pruned, with exact counters retained. Tests cover integer keys, a key named value, and partitioned TopK. Frequency bounds still do not certify TopK membership, so the accuracy gate correctly remains in force.
Final local validation: 690 focused tests passed, plus workspace format and all-target/all-feature Clippy checks.