Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions crates/frontend-promql/src/promql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,16 @@ fn selector_is_bucket(vs: &VectorSelector) -> bool {
fn walk_binary(bin: &BinaryExpr) -> Result<Unresolved> {
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,
Expand Down
7 changes: 5 additions & 2 deletions crates/frontend-promql/tests/observability/promql_corpus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:?}"
);
}
22 changes: 22 additions & 0 deletions crates/frontend-promql/tests/promql_lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
6 changes: 3 additions & 3 deletions docs/develop_docs/physical-compile-coverage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Loading