Skip to content

Commit 31ecc84

Browse files
zzylolclaude
andcommitted
fix(planner): SQL COUNT(*) candidates count rows instead of a sample value
Pass 1 built every COUNT(*) update from ColumnRef::SampleValue, which SQL rows (e.g. flows(ts, src_ip)) do not have. Exact Count accumulators failed to compose, and sketches hashed a non-existent `value` item. SQL COUNT(*) now adds a unit weight per row; a sketch hashes a non-null column, a grouping column first. The executor builds an exact count with a unit weight (Operator::unit_count_build), so the accumulator executes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 09fa09e commit 31ecc84

6 files changed

Lines changed: 240 additions & 9 deletions

File tree

‎crates/executor/src/operators/mod.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,10 @@ enum Kind {
135135
kind: planner_types::ir::operator::JoinKind,
136136
predicate: Box<crate::expressions::CompiledExpression>,
137137
},
138+
/// `value: None`: every row adds a unit weight (SQL `COUNT(*)`).
138139
SummaryBuild {
139140
family: SummaryFamilyType,
140-
value: usize,
141+
value: Option<usize>,
141142
time: Option<usize>,
142143
groups: Vec<usize>,
143144
},

‎crates/executor/src/operators/summary/mod.rs‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,32 @@ impl Operator {
168168
{
169169
return Err(invalid("counter summary requires a timestamp column"));
170170
}
171+
Self::grouped_build(input, family, Some(value), time, groups)
172+
}
173+
/// An exact count where every row adds one, whatever its values (SQL
174+
/// `COUNT(*)`); the rows need no value column.
175+
pub fn unit_count_build(
176+
input: SchemaRef,
177+
family: SummaryFamilyType,
178+
groups: Vec<usize>,
179+
) -> Result<Self, Error> {
180+
crate::values::validate_family(&family)?;
181+
validate_groups(&input, &groups)?;
182+
if !matches!(
183+
family,
184+
SummaryFamilyType::ExactAggregate(planner_types::ir::schema::ExactKind::Count, _)
185+
) {
186+
return Err(invalid("a unit-weight build needs an exact count"));
187+
}
188+
Self::grouped_build(input, family, None, None, groups)
189+
}
190+
fn grouped_build(
191+
input: SchemaRef,
192+
family: SummaryFamilyType,
193+
value: Option<usize>,
194+
time: Option<usize>,
195+
groups: Vec<usize>,
196+
) -> Result<Self, Error> {
171197
crate::capability::validate_summary_kernel(
172198
&family,
173199
&SummaryUpdate::column(ColumnRef::SampleValue),
@@ -439,7 +465,7 @@ pub(super) fn execute_merge<'a>(
439465
async fn build_summary(
440466
mut input: Input<'_, Batch>,
441467
family: &SummaryFamilyType,
442-
value: usize,
468+
value: Option<usize>,
443469
time: Option<usize>,
444470
groups: &[usize],
445471
emit_empty_global: bool,
@@ -496,7 +522,10 @@ async fn build_summary(
496522
states.get_mut(&key).expect("inserted group");
497523
// SQL aggregates ignore NULL samples while retaining the group.
498524
// A missing counter sample also contributes no observation.
499-
let value = &row[value];
525+
let value = match value {
526+
Some(value) => &row[value],
527+
None => &Value::Float64(1.0),
528+
};
500529
if matches!(value, Value::Null) {
501530
continue;
502531
}

‎crates/executor/src/operators/unchecked.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,10 @@ impl TryFrom<UncheckedOperator> for Operator {
155155
value,
156156
time,
157157
groups,
158-
} => Operator::summary_build(input(0)?, family, value, time, groups)?,
158+
} => match value {
159+
Some(value) => Operator::summary_build(input(0)?, family, value, time, groups)?,
160+
None => Operator::unit_count_build(input(0)?, family, groups)?,
161+
},
159162
Kind::KeyedSummaryBuild {
160163
family,
161164
value,

‎crates/executor/src/physical_planner/mod.rs‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -991,16 +991,26 @@ fn bind_operation(node: &PhysicalASAPDAGNode, inputs: &[SchemaRef]) -> Result<Op
991991
}
992992
crate::capability::validate_summary_kernel(family, update, grouping)
993993
.map_err(Error::Invalid)?;
994-
let SummaryInputExpr::Column(column) = &update.weight else {
995-
return Err(invalid(
996-
"summary update expression must be projected to a column",
997-
));
998-
};
999994
let PlannerReduction::Reduce(keys) = reduction else {
1000995
return Err(invalid(
1001996
"summary construction requires explicit grouping columns",
1002997
));
1003998
};
999+
let column = match &update.weight {
1000+
SummaryInputExpr::Column(column) => column,
1001+
SummaryInputExpr::Constant(weight) if *weight == 1.0 => {
1002+
return Operator::unit_count_build(
1003+
input.clone(),
1004+
family.clone(),
1005+
groups(input, keys)?,
1006+
)
1007+
}
1008+
_ => {
1009+
return Err(invalid(
1010+
"summary update expression must be projected to a column",
1011+
))
1012+
}
1013+
};
10041014
Operator::summary_build(
10051015
input.clone(),
10061016
family.clone(),
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
//! Pass 1 alternatives over SQL row sources compose, compile and execute.
2+
mod physical_common;
3+
use std::collections::BTreeMap;
4+
use std::rc::Rc;
5+
6+
use asap_executor::values::Value;
7+
use asap_frontend_sql::{lower_sql, SqlCatalog};
8+
use asap_logical_optimizer::pass1::logical_candidates::{
9+
compose_logical_candidate, enumerate_choices, enumerate_local_logical_candidates,
10+
LocalLogicalCandidates,
11+
};
12+
use asap_plan_selection::{plan_stages, PlanningModels};
13+
use asap_types::ir::schema::{DataType, Field, Schema};
14+
use asap_types::ir::{ASAPOp, Operator, OperatorNode, QueryRoot};
15+
use asap_types::types::AccuracyTarget;
16+
use asap_types::workload::{
17+
DataArrival, DataWorkload, Evidence, EvidenceSource, Predictability, QueryRecurrence, Rate,
18+
RootDemand,
19+
};
20+
21+
fn catalog() -> SqlCatalog {
22+
SqlCatalog::new().with_table(
23+
"flows",
24+
Schema::new(vec![
25+
Field::plain("ts", DataType::Timestamp, false),
26+
Field::plain("src_ip", DataType::Utf8, false),
27+
]),
28+
)
29+
}
30+
31+
fn target() -> AccuracyTarget {
32+
AccuracyTarget::EpsilonDelta {
33+
epsilon: 0.01,
34+
delta: 0.01,
35+
}
36+
}
37+
38+
fn rows() -> Vec<Vec<Value>> {
39+
["a", "a", "b", "a", "c", "b"]
40+
.into_iter()
41+
.map(|ip| vec![Value::Timestamp(0), Value::Utf8(ip.into())])
42+
.collect()
43+
}
44+
45+
async fn inventory(sql: &str) -> LocalLogicalCandidates<usize> {
46+
let root = lower_sql(sql, &catalog(), target()).await.unwrap();
47+
enumerate_local_logical_candidates(vec![(0, QueryRoot::Operator(root))], &BTreeMap::new())
48+
.unwrap()
49+
}
50+
51+
fn has_estimate(root: &Rc<OperatorNode>) -> bool {
52+
OperatorNode::reachable(root)
53+
.iter()
54+
.any(|n| matches!(n.operator, Operator::ASAP(ASAPOp::SummaryEstimate { .. })))
55+
}
56+
57+
/// `COUNT(*) … GROUP BY src_ip` (#509 Example 2's inner query) reads no
58+
/// sample value. Every candidate composes and compiles; the exact ones,
59+
/// the exact `Count` accumulator included, count rows per `src_ip`.
60+
#[tokio::test]
61+
async fn sql_count_star_group_by_candidates_compose_and_execute() {
62+
let inventory = inventory("SELECT src_ip, COUNT(*) AS c FROM flows GROUP BY src_ip").await;
63+
let choices = enumerate_choices(&inventory, usize::MAX);
64+
assert!(choices.len() > 2, "exact and summary candidates");
65+
let mut executed = 0;
66+
for choice in choices {
67+
let roots = compose_logical_candidate(&inventory, &choice)
68+
.unwrap_or_else(|e| panic!("{choice:?} composes: {e}"));
69+
let QueryRoot::Operator(root) = &roots[0].1 else {
70+
panic!("operator root")
71+
};
72+
physical_common::compile_physical_asap_dag(root)
73+
.unwrap_or_else(|e| panic!("{choice:?} compiles: {e}"));
74+
if has_estimate(root) {
75+
continue;
76+
}
77+
let mut rows = physical_common::execute_raw_rows(root, rows());
78+
rows.sort_by_key(|row| format!("{row:?}"));
79+
let counts: Vec<_> = rows
80+
.iter()
81+
.map(|row| match (&row[0], &row[1]) {
82+
(Value::Utf8(ip), Value::Int64(n)) => (ip.to_string(), *n),
83+
other => panic!("{choice:?}: unexpected row {other:?}"),
84+
})
85+
.collect();
86+
assert_eq!(
87+
counts,
88+
[("a".into(), 3), ("b".into(), 2), ("c".into(), 1)],
89+
"{choice:?}"
90+
);
91+
executed += 1;
92+
}
93+
assert_eq!(executed, 2, "pass-through and the exact Count accumulator");
94+
}
95+
96+
fn declared<T>(value: T) -> Evidence<T> {
97+
Evidence {
98+
value: Some(value),
99+
source: EvidenceSource::Declared,
100+
..Default::default()
101+
}
102+
}
103+
104+
/// #509 Example 2's design queries (its integer Q3 keeps `COUNT(*) GROUP BY
105+
/// src_ip` as a target): every candidate builds through Stages 1 and 2.
106+
/// Stage 3 may still reject one, e.g. for accuracy.
107+
#[tokio::test]
108+
async fn example2_design_candidates_all_build() {
109+
const QUERIES: [&str; 3] = [
110+
"SELECT COUNT(DISTINCT src_ip) FROM flows",
111+
"SELECT -SUM(p * LN(p)) FROM (SELECT COUNT(*) * 1.0 / SUM(COUNT(*)) OVER () AS p FROM flows GROUP BY src_ip)",
112+
"SELECT SQRT(SUM(c * c)) FROM (SELECT src_ip, COUNT(*) AS c FROM flows GROUP BY src_ip)",
113+
];
114+
let mut roots = vec![];
115+
for (i, sql) in QUERIES.into_iter().enumerate() {
116+
let root = lower_sql(sql, &catalog(), target()).await.unwrap();
117+
roots.push((i, QueryRoot::Operator(root)));
118+
}
119+
let demand = vec![
120+
RootDemand {
121+
accuracy: Some(target()),
122+
recurrence: QueryRecurrence::OneTime {
123+
invocations: 1,
124+
execute_at: None,
125+
},
126+
predictability: Predictability::default(),
127+
};
128+
roots.len()
129+
];
130+
let data = DataWorkload {
131+
arrival: DataArrival::ContinuouslyIngesting,
132+
ingestion_rate: declared(Rate(100_000.0)),
133+
input_cardinality: declared(10_000_000),
134+
..Default::default()
135+
};
136+
let run = plan_stages(roots, &demand, &data, PlanningModels::builtin(), 4096).unwrap();
137+
let enumeration = run.enumeration.unwrap();
138+
let unbuilt: Vec<_> = enumeration
139+
.selection
140+
.rejected
141+
.iter()
142+
.filter(|r| r.reason.starts_with("Stage "))
143+
.collect();
144+
assert!(enumeration.candidates.len() > 1);
145+
assert!(unbuilt.is_empty(), "{unbuilt:#?}");
146+
}

‎crates/logical-optimizer/src/pass1/logical_candidates.rs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,11 @@ fn summary_update(
689689
FieldDataType::Sketch(kind, _) => Some(kind.algorithm()),
690690
_ => None,
691691
};
692+
if let AggIntent::Count { .. } = intent {
693+
if child.closed && !child.has_promql_series_identity() {
694+
return sql_row_count_update(algorithm.is_some(), reduction, child);
695+
}
696+
}
692697
let weight = crate::pass1::replacement::summarised_input(intent, child)
693698
.map_err(|_| LogicalCandidateError::Unsupported("input column outside child schema"))?;
694699
Ok(match (intent, algorithm) {
@@ -746,6 +751,43 @@ fn summary_update(
746751
})
747752
}
748753

754+
/// SQL `COUNT(*)`: rows have no sample value, and every row counts, so each
755+
/// adds a unit weight. A sketch hashes an item per row; its bare count
756+
/// ignores the item's value, so any non-null column serves, a grouping
757+
/// column first.
758+
fn sql_row_count_update(
759+
sketch: bool,
760+
reduction: &Reduction,
761+
child: &Schema,
762+
) -> Result<SummaryUpdate, LogicalCandidateError> {
763+
let item = if sketch {
764+
let keys = reduction
765+
.group_keys()
766+
.filter(|keys| !keys.is_without())
767+
.into_iter()
768+
.flat_map(|keys| keys.iter().copied());
769+
let column = keys
770+
.chain(0..child.fields.len())
771+
.filter_map(|index| child.fields.get(index))
772+
.find(|field| field.is_plain() && !field.nullable)
773+
.ok_or(LogicalCandidateError::Unsupported(
774+
"a COUNT(*) sketch needs a non-null item column",
775+
))?;
776+
Some(SummaryInputExpr::Column(
777+
crate::pass1::replacement::column_ref(column),
778+
))
779+
} else {
780+
None
781+
};
782+
Ok(SummaryUpdate {
783+
item,
784+
weight: SummaryInputExpr::Constant(1.0),
785+
weight_domain: WeightDomain::NonNegative {
786+
proof: NonNegativeWeightProof::UnitCount,
787+
},
788+
})
789+
}
790+
749791
fn statistic(intent: &AggIntent) -> Result<SketchStatistic, LogicalCandidateError> {
750792
Ok(match intent {
751793
AggIntent::Quantile { q, .. } => SketchStatistic::Quantile { q: *q },

0 commit comments

Comments
 (0)