Skip to content

Commit 31bf28b

Browse files
zzylolclaude
andcommitted
docs(core): keep L3 TopK intent sketch-agnostic
AggIntent::TopK { k, accuracy } is a pure L3 intent (k most frequent by value, to accuracy ε), built from the lowering accuracy context exactly like Quantile/Cardinality. Its doc claimed it "is served by a dedicated sketch (SpaceSaving, CMS-with-heap) in one pass" and "never materialises- then-sorts" — physical-strategy language that belongs to L4 and is false for accuracy: Exact. Reframe the docs as intent-only and defer the exact-vs-sketch realisation to L4, matching the module's own rule that the HashAgg/SortAgg/SketchAgg choice is an L4 cost-aware decision. Keep the legitimate lowering-time split (heavy-hitter intent vs generic Sort+Limit). Doc/comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 179a5de commit 31bf28b

3 files changed

Lines changed: 16 additions & 12 deletions

File tree

crates/core/src/intent_algebra/agg_intent.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@
44
//! between `HashAgg` / `SortAgg` / `SketchAgg(KLL{k=200})` is an L4 cost-aware
55
//! decision, not encoded here.
66
//!
7-
//! `AggIntent::TopK` is a first-class *intent* — a dedicated heavy-hitter
8-
//! sketch (SpaceSaving, CMS-with-heap) computes it in one pass. Generic
9-
//! `ORDER BY value LIMIT k` stays as the `QueryExpr::Sort + Limit` operator
10-
//! pair. L1→L2→L3 lowering picks one or the other deterministically.
7+
//! `AggIntent::TopK` is a first-class *intent*: "the k most frequent keys by
8+
//! value, to accuracy ε." Like `Quantile`, the exact-vs-approximate
9+
//! realisation — an exact heap / sort+limit when `accuracy: Exact`, a
10+
//! heavy-hitter sketch when approximate — is an L4 cost-aware decision, not
11+
//! encoded here. The semantic distinction that *is* made at lowering is
12+
//! intent vs operator: a heavy-hitter aggregate becomes `TopK`, whereas a
13+
//! generic `ORDER BY value LIMIT k` stays as the `QueryExpr::Sort + Limit`
14+
//! operator pair.
1115
1216
use serde::{Deserialize, Serialize};
1317

@@ -66,8 +70,8 @@ pub enum AggIntent {
6670
q: f64,
6771
accuracy: AccuracyTarget,
6872
},
69-
/// Heavy-hitter top-k — served by a dedicated sketch in one pass. The
70-
/// group-by keys live on the enclosing `Aggregate.by`.
73+
/// Heavy-hitter top-k to the given accuracy. The group-by keys live on
74+
/// the enclosing `Aggregate.by`.
7175
TopK {
7276
k: usize,
7377
accuracy: AccuracyTarget,

crates/lower/src/promql.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
//! | `changes` / `resets` / `group` / `offset` / `@` | **rejected** — distinct semantics with no intent-algebra representation yet |
2525
//! | `OUTER by (dims) (…)` | `Aggregate.keys = dims` (→ positional `Aggregate.by` in L3; generic `topk by`/`bottomk` grouping → `Sort.partition_by`) |
2626
//! | `count by (d) (…)` | `Aggregate{[CountDistinct], …}` (→ `Cardinality`) |
27-
//! | `topk(k, count_over_time(…))` | `TopK{k, by}` (heavy-hitter, one pass) |
27+
//! | `topk(k, count_over_time(…))` | `TopK{k, by}` (heavy-hitter intent) |
2828
//! | `topk(k, <other>)` / `bottomk(k, …)` | `Sort{value} → Limit{k}` |
2929
//! | `m{f}` | `Filter(Source)` |
3030
//! | `a OP b` | `BinaryOp{vector_match}` |
@@ -400,10 +400,10 @@ fn build(inner: Inner, keys: Vec<ColumnRef>, outer: Outer) -> Result<L2> {
400400
}
401401
}),
402402
Outer::TopK { k, descending } => {
403-
// Heavy-hitter only when ranking by frequency (`count`): a dedicated
404-
// sketch serves it in one pass → first-class `TopK`. Any other
405-
// ranking (topk over avg/quantile, all bottomk) is generic
406-
// order-by-value + limit.
403+
// Heavy-hitter only when ranking by frequency (`count`): that is a
404+
// first-class aggregate intent → `TopK`. Any other ranking (topk
405+
// over avg/quantile, all bottomk) is a generic order-by-value +
406+
// limit and stays as the `Sort + Limit` operator pair.
407407
let heavy_hitter = descending && matches!(inner.func, Some(InnerFunc::Count));
408408
if heavy_hitter {
409409
// Preserve the Count intent in L3 so the intent algebra is

crates/lower/tests/promql_conformance.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ fn set_ops_lower_to_binaryop() {
561561

562562
#[test]
563563
fn topk_over_count_is_heavy_hitter() {
564-
// SEMANTICS: top-k by frequency → single-pass heavy-hitter sketch.
564+
// SEMANTICS: top-k by frequency → first-class heavy-hitter `TopK` intent.
565565
let qe = ok("topk(10, count_over_time(http_requests_total[1m]))");
566566
assert!(has(
567567
&qe,

0 commit comments

Comments
 (0)