Skip to content

PromQL: rate/increase and counter-derivative functions reject a sub-query argument (generalize #42) #55

Description

@zzylol

Summary

Range-vector functions accept a sub-query (expr[range:res]) wherever they accept a matrix selector — it's the other way (besides m[w]) to produce a range vector, and it is legitimate, parser-valid PromQL. Today only the *_over_time family accepts one (added in #42); rate/increase/irate and all the counter-derivative functions still reject a sub-query argument.

Current state (verified on main)

LOWERS    max_over_time(rate(m[5m])[1h:])        # *_over_time — done in #42
REJECTED  rate(sum(m)[5m:])                        # rate/increase/irate — NOT covered
REJECTED  increase(sum(m)[5m:])
REJECTED  irate(sum(m)[5m:])
REJECTED  changes(rate(m[5m])[1h:])                # counter-derivatives (#44) — NOT covered
REJECTED  delta(sum(m)[5m:])
REJECTED  deriv(sum(m)[10m:])
REJECTED  resets(rate(m[5m])[1h:])
REJECTED  idelta(sum(m)[5m:])
REJECTED  predict_linear(sum(m)[1h:], 3600)
REJECTED  double_exponential_smoothing(sum(m)[10m:], 0.5, 0.3)

All reject with unsupported feature: expected a range-vector (matrix) argument, got Discriminant(4) — i.e. our lowering refuses the sub-query; the parser accepts it (Discriminant(4) = Subquery). So these are valid PromQL queries Prometheus executes, e.g. changes(rate(m[5m])[1h:]) = "how many times the 5m rate changed over the past hour, per series".

Root cause

extract_matrix (crates/lower/src/promql.rs) only accepts a (parenthesised) MatrixSelector. #42 special-cased the *_over_time reducers to lower a sub-query argument into a PromQLSubquery and reduce it per series (over_time_reducer + the is_subquery branch in walk_call). rate/increase/irate and the counter-derivatives (changes/delta/idelta/deriv/resets/predict_linear/double_exponential_smoothing) go through lower_inner_callextract_matrix and were never wired into that path.

Proposed fix

Generalize the #42 sub-query path to the whole range-vector-function family: when the matrix argument slot is a Subquery, lower it via walk into a PromQLSubquery and wrap it in the function's per-series intent (Rate/Increase/Changes/Delta/…), instead of calling extract_matrix. The L3 per-series marker already covers a Subquery child (added in #42), so the schema side needs no change — only the front end.

Watch-outs:

  • rate/increase carry their window in the AggFunc variant (no L2 Window node); over a sub-query the "window" is the sub-query's own range, so the shape is Aggregate{Rate} → PromQLSubquery{range,res} → <inner> (no separate TimeRange — the sub-query is the range marker). Confirm is_per_series/schema derivation is happy with Rate directly over a Subquery.
  • Thread scalar params (predict_linear horizon, double_exponential_smoothing factors) as usual.

Tests

  • Pinned today by counter_derivative_over_a_subquery_is_rejected__GAP (crates/lower/tests/promql_conformance.rs) — flip it to a passing conformance test when implemented.
  • Add positive tests for rate/increase over a sub-query alongside the existing over_time_of_subquery_* tests.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestpromqlPromQL front-end lowering (L1→L2)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions