Skip to content

Commit 29e6dc6

Browse files
zzylolzz_yclaude
authored
fix(promql): represent set-op default matching as ignoring([]), not on([]) (#68) (#74)
The parser attaches a default modifier to every `and`/`or`/`unless`, whose `matching` is `None`. `walk_binary` mapped that to `VectorMatch{On, []}` — byte-identical to an explicit `on()`. So `a and b` and `a and on() b` lowered to the same tree, and the default "match on all shared labels" was misrepresented as "match on the empty label set". The default is exactly `ignoring([])` (ignore no labels ⇒ match on all shared labels). Map the `None` arm to `Ignoring([])` instead: - `a and b` → Ignoring([]) - `a and on() b` → On([]) (now distinct) - `a and ignoring() b` → Ignoring([]) (now correctly equal to default) Added `set_op_default_match_is_ignoring_empty_not_on_empty` (distinct-vs-on, equal-to-ignoring). Full workspace suite green; clippy --all-targets clean. Co-authored-by: zz_y <zz_y@node0.zz-y-308294.softmeasure-pg0.clemson.cloudlab.us> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d38831d commit 29e6dc6

2 files changed

Lines changed: 22 additions & 1 deletion

File tree

‎crates/frontend-promql/src/promql.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,14 @@ fn walk_binary(bin: &BinaryExpr) -> Result<L2> {
388388
let (kind, labels) = match &m.matching {
389389
Some(LabelModifier::Include(ls)) => (VectorMatchKind::On, ls.labels.clone()),
390390
Some(LabelModifier::Exclude(ls)) => (VectorMatchKind::Ignoring, ls.labels.clone()),
391-
None => (VectorMatchKind::On, vec![]),
391+
// No explicit `on(…)`/`ignoring(…)` — the parser attaches a default
392+
// modifier to every set op (`and`/`or`/`unless`). The default is
393+
// "match on all shared labels", which is exactly `ignoring([])`
394+
// (ignore no labels). Representing it as `Ignoring([])` — not
395+
// `On([])` — keeps it distinct from an explicit `on()` (match on the
396+
// empty label set) while making it correctly equal to an explicit
397+
// `ignoring()` (issue #68).
398+
None => (VectorMatchKind::Ignoring, vec![]),
392399
};
393400
let grouping = match &m.card {
394401
VectorMatchCardinality::ManyToOne(ls) => Some(VectorGrouping {

‎crates/frontend-promql/tests/promql_equivalence.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,20 @@ fn rate_and_irate_share_the_same_intent() {
141141
assert_equiv(&["rate(m[5m])", "irate(m[5m])"]);
142142
}
143143

144+
#[test]
145+
fn set_op_default_match_is_ignoring_empty_not_on_empty() {
146+
// Issue #68. A set op's *default* matching ("match on all shared labels") is
147+
// `ignoring([])`, NOT `on([])`. So:
148+
// - default `a and b` must stay DISTINCT from explicit `a and on() b`
149+
// (which matches on the empty label set), and
150+
// - default `a and b` must EQUAL explicit `a and ignoring() b`
151+
// (ignore no labels ⇒ match on all shared labels).
152+
assert_distinct("a and b", "a and on() b");
153+
assert_distinct("a or b", "a or on() b");
154+
assert_equiv(&["a and b", "a and ignoring() b"]);
155+
assert_equiv(&["a unless b", "a unless ignoring() b"]);
156+
}
157+
144158
// ─────────────────────────────────────────────────────────────────────────────
145159
// 4. Distinct semantics we cannot faithfully represent are REJECTED, not
146160
// silently merged into a wrong intent. (Each previously mislowered.)

0 commit comments

Comments
 (0)