Skip to content

Commit c402f0b

Browse files
committed
fix(cost): distinguish PromQL binary operand modes
1 parent 2280895 commit c402f0b

4 files changed

Lines changed: 236 additions & 29 deletions

File tree

‎crates/asap-aware-mapping/src/analytical_cost.rs‎

Lines changed: 122 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize};
1313

1414
use crate::analytical_statistics::{
1515
validate_comparison_scopes, ComparisonScope, OperatorStatistics, OperatorStatisticsProvider,
16-
SourceCoverage,
16+
PromqlBinaryOperandMode, SourceCoverage,
1717
};
1818

1919
pub const ANALYTICAL_MODEL_VERSION: &str = "analytical-resource-at-rest-v1";
@@ -465,9 +465,9 @@ pub fn estimate_operator(
465465
let output = statistics.output;
466466
if matches!(operator, PhysicalOperator::PromqlScalarLeaf) {
467467
let promql = require_promql_statistics(&statistics, 0)?;
468-
if promql.output_series > output.rows && output.rows > 0 {
468+
if promql.output_series != 0 || output.rows != promql.evaluation_steps {
469469
return Err(AnalyticalCostError::InconsistentOperatorStatistics(
470-
"output series exceed output rows",
470+
"PromQL scalar leaf must emit one scalar row per evaluation step",
471471
));
472472
}
473473
return Ok(ResourceEstimate {
@@ -601,27 +601,29 @@ pub fn estimate_operator(
601601
PhysicalOperator::PromqlVectorBinary => {
602602
let promql = require_promql_statistics(&statistics, 2)?;
603603
let right = input(1)?;
604-
let scalar_vector = matches!(operator, PhysicalOperator::PromqlVectorBinary)
605-
&& promql.input_series.contains(&0);
606-
let matching_bytes = if scalar_vector {
607-
per_row_width(output.rows, output.bytes)?
608-
} else {
609-
let build_side = statistics
610-
.hash_join_build_side
611-
.ok_or(AnalyticalCostError::MissingOrZero("hash_join_build_side"))?;
612-
let key_bytes = statistics
613-
.key_bytes
614-
.ok_or(AnalyticalCostError::MissingOrZero("key_bytes"))?;
615-
let build_series = match build_side {
616-
HashJoinBuildSide::Left => promql.input_series[0],
617-
HashJoinBuildSide::Right => promql.input_series[1],
618-
};
619-
checked_bytes(&[
620-
build_series,
621-
key_bytes
622-
.checked_add(16)
623-
.ok_or(AnalyticalCostError::Overflow)?,
624-
])?
604+
let mode = validate_promql_binary(&statistics)?;
605+
let matching_bytes = match mode {
606+
PromqlBinaryOperandMode::VectorScalar | PromqlBinaryOperandMode::ScalarVector => {
607+
per_row_width(output.rows, output.bytes)?
608+
}
609+
PromqlBinaryOperandMode::VectorVector => {
610+
let build_side = statistics
611+
.hash_join_build_side
612+
.ok_or(AnalyticalCostError::MissingOrZero("hash_join_build_side"))?;
613+
let key_bytes = statistics
614+
.key_bytes
615+
.ok_or(AnalyticalCostError::MissingOrZero("key_bytes"))?;
616+
let build_series = match build_side {
617+
HashJoinBuildSide::Left => promql.input_series[0],
618+
HashJoinBuildSide::Right => promql.input_series[1],
619+
};
620+
checked_bytes(&[
621+
build_series,
622+
key_bytes
623+
.checked_add(16)
624+
.ok_or(AnalyticalCostError::Overflow)?,
625+
])?
626+
}
625627
};
626628
ResourceEstimate {
627629
cpu_ops: left.rows as f64 + right.rows as f64 + output.rows as f64,
@@ -756,6 +758,49 @@ fn validate_promql_bridge(
756758
Ok(())
757759
}
758760

761+
fn validate_promql_binary(
762+
statistics: &OperatorStatistics,
763+
) -> Result<PromqlBinaryOperandMode, AnalyticalCostError> {
764+
let promql = require_promql_statistics(statistics, 2)?;
765+
let mode = promql
766+
.binary_operand_mode
767+
.ok_or(AnalyticalCostError::MissingOrStale(
768+
"promql_binary_operand_mode",
769+
))?;
770+
match mode {
771+
PromqlBinaryOperandMode::VectorVector => {
772+
if statistics.hash_join_build_side.is_none() || statistics.key_bytes.is_none() {
773+
return Err(AnalyticalCostError::MissingOrStale(
774+
"vector_binary_label_match_statistics",
775+
));
776+
}
777+
}
778+
PromqlBinaryOperandMode::VectorScalar => {
779+
if promql.input_series[1] != 0
780+
|| statistics.inputs[1].rows != promql.evaluation_steps
781+
|| statistics.hash_join_build_side.is_some()
782+
|| statistics.key_bytes.is_some()
783+
{
784+
return Err(AnalyticalCostError::InconsistentOperatorStatistics(
785+
"vector/scalar binary evidence has an invalid scalar edge or label-match state",
786+
));
787+
}
788+
}
789+
PromqlBinaryOperandMode::ScalarVector => {
790+
if promql.input_series[0] != 0
791+
|| statistics.inputs[0].rows != promql.evaluation_steps
792+
|| statistics.hash_join_build_side.is_some()
793+
|| statistics.key_bytes.is_some()
794+
{
795+
return Err(AnalyticalCostError::InconsistentOperatorStatistics(
796+
"scalar/vector binary evidence has an invalid scalar edge or label-match state",
797+
));
798+
}
799+
}
800+
}
801+
Ok(mode)
802+
}
803+
759804
fn require_promql_statistics(
760805
statistics: &OperatorStatistics,
761806
inputs: usize,
@@ -1202,6 +1247,7 @@ mod tests {
12021247
window_samples_per_series: None,
12031248
subquery_steps: None,
12041249
scalar_ops_per_row: None,
1250+
binary_operand_mode: None,
12051251
});
12061252
statistics
12071253
};
@@ -1246,6 +1292,58 @@ mod tests {
12461292
));
12471293
}
12481294

1295+
#[test]
1296+
fn promql_binary_mode_distinguishes_empty_vectors_from_scalars() {
1297+
use crate::analytical_statistics::{PromqlBinaryOperandMode, PromqlOperatorStatistics};
1298+
1299+
let empty = EdgeStatistics { rows: 0, bytes: 0 };
1300+
let mut vector_vector = OperatorStatistics {
1301+
key_bytes: Some(16),
1302+
hash_join_build_side: Some(HashJoinBuildSide::Right),
1303+
promql: Some(PromqlOperatorStatistics {
1304+
input_series: vec![0, 0],
1305+
output_series: 0,
1306+
evaluation_steps: 10,
1307+
window_samples_per_series: None,
1308+
subquery_steps: None,
1309+
scalar_ops_per_row: None,
1310+
binary_operand_mode: Some(PromqlBinaryOperandMode::VectorVector),
1311+
}),
1312+
..statistics(vec![empty, empty], empty)
1313+
};
1314+
assert!(
1315+
estimate_operator(PhysicalOperator::PromqlVectorBinary, vector_vector.clone()).is_ok()
1316+
);
1317+
1318+
vector_vector.hash_join_build_side = None;
1319+
assert_eq!(
1320+
estimate_operator(PhysicalOperator::PromqlVectorBinary, vector_vector),
1321+
Err(AnalyticalCostError::MissingOrStale(
1322+
"vector_binary_label_match_statistics"
1323+
))
1324+
);
1325+
}
1326+
1327+
#[test]
1328+
fn promql_scalar_leaf_requires_one_row_per_evaluation_step() {
1329+
use crate::analytical_statistics::PromqlOperatorStatistics;
1330+
1331+
let mut scalar = statistics(vec![], EdgeStatistics { rows: 9, bytes: 72 });
1332+
scalar.promql = Some(PromqlOperatorStatistics {
1333+
input_series: vec![],
1334+
output_series: 0,
1335+
evaluation_steps: 10,
1336+
window_samples_per_series: None,
1337+
subquery_steps: None,
1338+
scalar_ops_per_row: None,
1339+
binary_operand_mode: None,
1340+
});
1341+
assert!(matches!(
1342+
estimate_operator(PhysicalOperator::PromqlScalarLeaf, scalar),
1343+
Err(AnalyticalCostError::InconsistentOperatorStatistics(_))
1344+
));
1345+
}
1346+
12491347
#[test]
12501348
fn comparison_rejects_different_snapshot_predicate_time_or_horizon() {
12511349
use std::rc::Rc;

‎crates/asap-aware-mapping/src/analytical_lowering.rs‎

Lines changed: 94 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ use crate::analytical_cost::{
88
AnalyticalCostError, ExecutionMultiplicity, PhysicalDagNode, PhysicalOperator,
99
};
1010
use crate::analytical_statistics::{
11-
ComparisonScope, EdgeStatistics, OperatorStatistics, OperatorStatisticsProvider, SourceCoverage,
11+
ComparisonScope, EdgeStatistics, OperatorStatistics, OperatorStatisticsProvider,
12+
PromqlBinaryOperandMode, SourceCoverage,
1213
};
1314

1415
/// A lowered physical DAG and the node whose output is the query result.
@@ -605,6 +606,19 @@ pub fn lower_query_physical_dag(
605606
"PromQL binary inputs do not match child outputs",
606607
));
607608
}
609+
let operand_mode = match (left_scalar, right_scalar) {
610+
(false, false) => PromqlBinaryOperandMode::VectorVector,
611+
(false, true) => PromqlBinaryOperandMode::VectorScalar,
612+
(true, false) => PromqlBinaryOperandMode::ScalarVector,
613+
(true, true) => unreachable!("scalar/scalar was rejected above"),
614+
};
615+
if require_promql_statistics(statistics, 2)?.binary_operand_mode
616+
!= Some(operand_mode)
617+
{
618+
return Err(AnalyticalCostError::InconsistentOperatorStatistics(
619+
"PromQL binary operand mode disagrees with its logical operands",
620+
));
621+
}
608622
if left_scalar || right_scalar {
609623
if vector_match.is_some()
610624
|| statistics.hash_join_build_side.is_some()
@@ -895,7 +909,10 @@ fn require_operator_statistics(
895909
return invalid("only Scan may charge source bytes");
896910
}
897911
if matches!(operator, PhysicalOperator::PromqlScalarLeaf) {
898-
require_promql_statistics(statistics, 0)?;
912+
let promql = require_promql_statistics(statistics, 0)?;
913+
if promql.output_series != 0 || statistics.output.rows != promql.evaluation_steps {
914+
return invalid("PromQL scalar leaf must emit one scalar row per evaluation step");
915+
}
899916
return Ok(());
900917
}
901918
let input = statistics.inputs.first().copied().ok_or(
@@ -982,7 +999,43 @@ fn require_operator_statistics(
982999
}
9831000
}
9841001
PhysicalOperator::PromqlVectorBinary => {
985-
require_promql_statistics(statistics, 2)?;
1002+
let promql = require_promql_statistics(statistics, 2)?;
1003+
let mode = promql
1004+
.binary_operand_mode
1005+
.ok_or(AnalyticalCostError::MissingOrStale(
1006+
"promql_binary_operand_mode",
1007+
))?;
1008+
match mode {
1009+
PromqlBinaryOperandMode::VectorVector => {
1010+
if statistics.hash_join_build_side.is_none() || statistics.key_bytes.is_none() {
1011+
return Err(AnalyticalCostError::MissingOrStale(
1012+
"vector_binary_label_match_statistics",
1013+
));
1014+
}
1015+
}
1016+
PromqlBinaryOperandMode::VectorScalar => {
1017+
if promql.input_series[1] != 0
1018+
|| statistics.inputs[1].rows != promql.evaluation_steps
1019+
|| statistics.hash_join_build_side.is_some()
1020+
|| statistics.key_bytes.is_some()
1021+
{
1022+
return invalid(
1023+
"vector/scalar binary evidence has an invalid scalar edge or label-match state",
1024+
);
1025+
}
1026+
}
1027+
PromqlBinaryOperandMode::ScalarVector => {
1028+
if promql.input_series[0] != 0
1029+
|| statistics.inputs[0].rows != promql.evaluation_steps
1030+
|| statistics.hash_join_build_side.is_some()
1031+
|| statistics.key_bytes.is_some()
1032+
{
1033+
return invalid(
1034+
"scalar/vector binary evidence has an invalid scalar edge or label-match state",
1035+
);
1036+
}
1037+
}
1038+
}
9861039
}
9871040
PhysicalOperator::PromqlInfoEnrich => {
9881041
let promql = require_promql_statistics(statistics, 2)?;
@@ -1441,6 +1494,7 @@ mod tests {
14411494
window_samples_per_series: None,
14421495
subquery_steps: None,
14431496
scalar_ops_per_row: None,
1497+
binary_operand_mode: None,
14441498
});
14451499
statistics
14461500
}
@@ -2333,10 +2387,15 @@ mod tests {
23332387
let empty_scope = scope(vec![]);
23342388
let scalar = Rc::new(QueryExpr::promql_scalar(5.0));
23352389
let scalar_statistics = with_promql(statistics(vec![], edge(10, 80)), vec![], 0, 10);
2336-
let provided = HashMap::from([("query-0".into(), evidence(scalar_statistics))]);
2390+
let mut provided = HashMap::from([("query-0".into(), evidence(scalar_statistics))]);
23372391
let dag = lower_query_physical_dag(&scalar, &empty_scope, &scripted(&provided)).unwrap();
23382392
assert_eq!(dag.nodes[0].operator, PhysicalOperator::PromqlScalarLeaf);
23392393
assert!(estimate_physical_dag(&dag.nodes, &dag.root, &empty_scope, &dag.evidence).is_ok());
2394+
provided.get_mut("query-0").unwrap().statistics.output = edge(9, 72);
2395+
assert!(matches!(
2396+
lower_query_physical_dag(&scalar, &empty_scope, &scripted(&provided)),
2397+
Err(AnalyticalCostError::InconsistentOperatorStatistics(_))
2398+
));
23402399

23412400
let timestamp = Rc::new(QueryExpr::EvalTimestamp);
23422401
let timestamp_statistics = with_promql(statistics(vec![], edge(10, 80)), vec![], 0, 10);
@@ -2549,12 +2608,14 @@ mod tests {
25492608
);
25502609
vector_statistics.source_scan_bytes = 8_000;
25512610
let scalar = with_promql(statistics(vec![], edge(10, 80)), vec![], 0, 10);
2552-
let binary = with_promql(
2611+
let mut binary = with_promql(
25532612
statistics(vec![edge(1_000, 16_000), edge(10, 80)], edge(1_000, 16_000)),
25542613
vec![100, 0],
25552614
100,
25562615
10,
25572616
);
2617+
binary.promql.as_mut().unwrap().binary_operand_mode =
2618+
Some(PromqlBinaryOperandMode::VectorScalar);
25582619
let provided = HashMap::from([
25592620
("query-1".into(), evidence(vector_statistics)),
25602621
("query-2".into(), evidence(scalar)),
@@ -2649,6 +2710,8 @@ mod tests {
26492710
);
26502711
binary.key_bytes = Some(32);
26512712
binary.hash_join_build_side = Some(HashJoinBuildSide::Right);
2713+
binary.promql.as_mut().unwrap().binary_operand_mode =
2714+
Some(PromqlBinaryOperandMode::VectorVector);
26522715
let provided = HashMap::from([
26532716
("query-1".into(), evidence(left)),
26542717
("query-2".into(), evidence(right)),
@@ -2659,6 +2722,32 @@ mod tests {
26592722
assert_eq!(estimate.cpu_ops, 3_400.0);
26602723
assert_eq!(estimate.scan_bytes, 12_000);
26612724

2725+
let mut empty = provided.clone();
2726+
for id in ["query-1", "query-2"] {
2727+
let evidence = empty.get_mut(id).unwrap();
2728+
evidence.statistics.inputs[0] = edge(0, 0);
2729+
evidence.statistics.output = edge(0, 0);
2730+
let promql = evidence.statistics.promql.as_mut().unwrap();
2731+
promql.input_series[0] = 0;
2732+
promql.output_series = 0;
2733+
evidence.output_buffer_bytes = 0;
2734+
}
2735+
let evidence = empty.get_mut("query-0").unwrap();
2736+
evidence.statistics.inputs = vec![edge(0, 0), edge(0, 0)];
2737+
evidence.statistics.output = edge(0, 0);
2738+
let promql = evidence.statistics.promql.as_mut().unwrap();
2739+
promql.input_series = vec![0, 0];
2740+
promql.output_series = 0;
2741+
evidence.output_buffer_bytes = 0;
2742+
let empty_dag = lower_query_physical_dag(&root, &scope, &scripted(&empty)).unwrap();
2743+
assert!(estimate_physical_dag(
2744+
&empty_dag.nodes,
2745+
&empty_dag.root,
2746+
&scope,
2747+
&empty_dag.evidence
2748+
)
2749+
.is_ok());
2750+
26622751
let mut missing = provided;
26632752
missing
26642753
.get_mut("query-0")

‎crates/asap-aware-mapping/src/analytical_statistics.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,17 @@ pub struct PromqlOperatorStatistics {
229229
pub window_samples_per_series: Option<u64>,
230230
pub subquery_steps: Option<u64>,
231231
pub scalar_ops_per_row: Option<u64>,
232+
/// Physical operand shape for a PromQL binary operator. Series cardinality
233+
/// cannot encode this because a vector may legitimately contain zero series.
234+
#[serde(default)]
235+
pub binary_operand_mode: Option<PromqlBinaryOperandMode>,
236+
}
237+
238+
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
239+
pub enum PromqlBinaryOperandMode {
240+
VectorVector,
241+
VectorScalar,
242+
ScalarVector,
232243
}
233244

234245
/// Resolves physical statistics and owns their catalog/observation freshness.

‎docs/design_docs/asap-aware-mapping/analytical-resource-cost.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ OperatorStatistics {
161161
promql: PromqlOperatorStatistics {
162162
input_series, output_series, evaluation_steps,
163163
window_samples_per_series, subquery_steps, scalar_ops_per_row,
164+
binary_operand_mode,
164165
},
165166
}
166167
```
@@ -353,6 +354,14 @@ both input and output contain one row per evaluation step. A vector-to-scalar
353354
bridge emits zero series and exactly one scalar row per evaluation step; its
354355
input-series count must equal the vector child's output-series count.
355356

357+
PromQL binary evidence also records an explicit physical operand mode:
358+
vector/vector, vector/scalar, or scalar/vector. Cardinality does not determine
359+
the mode because a vector may legitimately contain zero series. Lowering
360+
checks the mode against the canonical logical operands. Vector/vector always
361+
requires label-match state, including for an empty vector; only the two
362+
scalar/vector modes omit it. A scalar leaf has no inputs or series and emits
363+
exactly one scalar row per evaluation step.
364+
356365
`info()` resolves its default `target_info` metric, or one exact `__name__`
357366
matcher, to a concrete existing `Source::TimeSeries` coverage. Its right side
358367
is an ordinary physical Scan with its own statistics, buffer, snapshot, and

0 commit comments

Comments
 (0)