Skip to content
Merged
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
32 changes: 32 additions & 0 deletions crates/frontend-promql/src/promql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,11 @@ fn walk(expr: &Expr) -> Result<L2> {
Expr::Call(call) if is_typeconv_fn(call.func.name) => walk_typeconv(call),
Expr::Call(call) if is_label_fn(call.func.name) => walk_label(call),
Expr::Call(call) if is_sort_fn(call.func.name) => walk_sort(call),
// A bare `min_of`/`max_of(consts…)` scalar query folds to a `Scalar`
// leaf; a non-constant argument makes `num_expr` fail → rejected (#89).
Expr::Call(call) if is_scalar_reducer_fn(call.func.name) => {
Ok(L2::Scalar(num_expr(expr)?))
}
Expr::Call(call) => walk_call(call),
Expr::Binary(bin) => walk_binary(bin),
Expr::Paren(p) => walk(&p.expr),
Expand Down Expand Up @@ -1433,13 +1438,40 @@ fn num_expr(expr: &Expr) -> Result<f64> {
))
}
}
// `min_of`/`max_of` are n-ary *scalar* reducers (issue #89). Fold them
// when every argument is itself a constant scalar — this is the only
// form the intent algebra can hold (there is no scalar min/max node). A
// non-constant argument (`min_of(step(), 1s)`) fails the recursive fold
// and propagates the error, so it stays rejected. `f64::min`/`max`
// ignore NaN, matching PromQL's `min`/`max` NaN semantics.
Expr::Call(c) if is_scalar_reducer_fn(c.func.name) => {
let reduce = if c.func.name == "min_of" {
f64::min
} else {
f64::max
};
c.args
.args
.iter()
.map(|a| num_expr(a))
.reduce(|acc, v| Ok(reduce(acc?, v?)))
.ok_or_else(|| {
LoweringError::MissingArgument(format!("{} needs an argument", c.func.name))
})?
}
other => Err(LoweringError::InvalidParameter(format!(
"expected a numeric scalar, got {:?}",
std::mem::discriminant(other)
))),
}
}

/// The n-ary scalar min/max reducers, foldable when all arguments are constant
/// scalars (issue #89).
fn is_scalar_reducer_fn(name: &str) -> bool {
matches!(name, "min_of" | "max_of")
}

/// A `BinaryOp` operand: fold a pure-scalar expression (`5`, `10*1024*1024`) to
/// a `Scalar` leaf, otherwise walk it as a vector (issue #35).
fn scalar_or_vector(expr: &Expr) -> Result<L2> {
Expand Down
38 changes: 31 additions & 7 deletions crates/frontend-promql/tests/promql_conformance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1573,11 +1573,35 @@ fn sort_by_label_desc_is_descending() {
}

#[test]
fn min_of_max_of_are_rejected__GAP() {
// `min_of`/`max_of` are n-ary *scalar* reducers, almost always nested with
// the `step()`/`range()` scalar helpers inside range/offset positions that
// are themselves unsupported. They need a scalar-reduction node — deferred
// to #89, not mislowered.
let _ = rejected("min_of(1, 2, 3)");
let _ = rejected("max_of(1, 2)");
fn min_of_max_of_fold_constant_scalars() {
// `min_of`/`max_of` are n-ary scalar reducers. When every argument is a
// constant they constant-fold to a `Scalar` leaf, just like scalar
// arithmetic (#35) — the only form the intent algebra can hold (#89).
assert!(matches!(ok("min_of(3, 5)"), QueryExpr::Scalar(v) if v == 3.0));
assert!(matches!(ok("max_of(3, 5)"), QueryExpr::Scalar(v) if v == 5.0));
assert!(matches!(ok("min_of(-2, -5)"), QueryExpr::Scalar(v) if v == -5.0));
// Nested folds and use as a threshold operand.
assert!(matches!(ok("max_of(min_of(2, 3), 10)"), QueryExpr::Scalar(v) if v == 10.0));
let qe = ok("up > max_of(1, 2)");
let QueryExpr::BinaryOp { rhs, .. } = &qe else {
panic!("{qe:?}")
};
assert!(matches!(rhs.as_ref(), QueryExpr::Scalar(v) if *v == 2.0));
}

#[test]
fn min_of_max_of_ignore_nan_like_the_min_max_aggregators() {
// A NaN argument is skipped (Prometheus `min`/`max` NaN semantics).
assert!(matches!(ok("max_of(3, NaN)"), QueryExpr::Scalar(v) if v == 3.0));
assert!(matches!(ok("min_of(NaN, 3)"), QueryExpr::Scalar(v) if v == 3.0));
}

#[test]
fn non_constant_min_of_max_of_is_rejected__GAP() {
// A dynamic argument (`step()` — itself unsupported, #89) can't be folded to
// a constant and there is no scalar min/max node, so it stays rejected
// rather than mislowered. These forms also only appear inside unsupported
// dynamic range / offset positions in the corpus.
let _ = rejected("min_of(step(), 1s)");
let _ = rejected("max_of(min_of(step() + 1, 1h), 1ms)");
}
Loading