diff --git a/crates/frontend-promql/src/promql.rs b/crates/frontend-promql/src/promql.rs index 6564003d..a7d83e76 100644 --- a/crates/frontend-promql/src/promql.rs +++ b/crates/frontend-promql/src/promql.rs @@ -1274,6 +1274,16 @@ fn selector_is_bucket(vs: &VectorSelector) -> bool { fn walk_binary(bin: &BinaryExpr) -> Result { let lhs = scalar_or_vector(&bin.lhs)?; let rhs = scalar_or_vector(&bin.rhs)?; + // `VectorMatch` has no fill field; dropping fill would change which series + // are emitted and their values, so the query must fall back to exact + // execution instead. + if let Some(m) = &bin.modifier { + if m.fill_values.lhs.is_some() || m.fill_values.rhs.is_some() { + return Err(LoweringError::UnsupportedFeature(format!( + "`fill` vector-matching modifier: `{bin}`" + ))); + } + } let op = match (binop(bin.op.id())?, bin.return_bool()) { (BinaryOpKind::Compare(op), true) => BinaryOpKind::CompareBool(op), (op, _) => op, diff --git a/crates/frontend-promql/tests/observability/promql_corpus.rs b/crates/frontend-promql/tests/observability/promql_corpus.rs index beb16d61..f265edb4 100644 --- a/crates/frontend-promql/tests/observability/promql_corpus.rs +++ b/crates/frontend-promql/tests/observability/promql_corpus.rs @@ -155,13 +155,16 @@ fn lowering_is_total_over_the_entire_corpus() { // rather than pin an exact count — ratchet them up as coverage lands. // // The 235 unparseable are parser-fork gaps (issue #108); the rejections are - // lowering gaps (#109). Both shrink over time, so these floors only ever rise. + // lowering gaps (#109). Both shrink over time, so these floors normally only + // rise. Exception: the testdata floor was lowered to the measured 1485 when + // the 44 `fill` vector-matching queries became rejected rather than + // silently lowered without their fill semantics. assert!( docs.lowered >= 47, "docs lowering coverage regressed: {docs:?}" ); assert!( - td.lowered >= 1495, + td.lowered >= 1485, "testdata lowering coverage regressed: {td:?}" ); } diff --git a/crates/frontend-promql/tests/promql_lowering.rs b/crates/frontend-promql/tests/promql_lowering.rs index a71cb5ba..eea38873 100644 --- a/crates/frontend-promql/tests/promql_lowering.rs +++ b/crates/frontend-promql/tests/promql_lowering.rs @@ -864,6 +864,28 @@ fn pathologically_nested_query_is_rejected_not_stack_overflow() { assert!(format!("{err}").contains("nesting"), "got {err}"); } +// Behavior: every parser-accepted `fill` modifier form is rejected with a +// fill-specific lowering error rather than silently dropped. +#[test] +fn fill_modifiers_are_rejected_not_ignored() { + for q in [ + "a + fill(0) b", + "a + fill_left(1) b", + "a + fill_right(2) b", + "a + fill_left(1) fill_right(2) b", + "a + fill_right(2) fill_left(1) b", + "a + on(job) fill(0) b", + "a * ignoring(instance) group_left(env) fill_right(0) b", + "a > bool on(job) fill(0) b", + "sum(a - on(job) group_right fill_left(0) b)", + ] { + match lower_promql(q, AccuracyTarget::Exact) { + Err(LoweringError::UnsupportedFeature(m)) if m.contains("`fill`") => {} + other => panic!("expected fill rejection for {q:?}, got {other:?}"), + } + } +} + // ── accuracy propagation ────────────────────────────────────────────────────── #[test] diff --git a/docs/develop_docs/physical-compile-coverage.md b/docs/develop_docs/physical-compile-coverage.md index 4d6dc44c..32f93292 100644 --- a/docs/develop_docs/physical-compile-coverage.md +++ b/docs/develop_docs/physical-compile-coverage.md @@ -229,6 +229,6 @@ In order of backend usage: aggregated (label-column) rows. The logical output schema, which is the left side's, has no column for them. 9. An equal-label-set check for inner subquery functions, per step. -10. `fill`, `fill_left`, and `fill_right` matching modifiers. The frontend - still ignores them. Rejecting them drops 27 corpus queries below the - lowering floor, so that change needs its own decision. + +`fill`, `fill_left`, and `fill_right` matching modifiers are rejected by the +frontend (#494); they are never silently ignored.