Skip to content

feat(promql): lower offset / @ time-shift modifiers to a TimeShift node (#40) - #106

Merged
zzylol merged 1 commit into
mainfrom
feat/40-offset-at-modifiers
Jul 6, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/40-offset-at-modifiers

Conversation

@zzylol

@zzylol zzylol commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Closes #40.

The idea

m offset 1h and m @ 1609746000 were rejected outright: unlike without (#39, missing catalog metadata), this was blocked on a missing IR node — nothing could carry the shift amount. But offset/@ don't need a schema change: neither modifier alters a selector's columns, they only move when it is evaluated. So the faithful representation is a pass-through wrapper over the selector — exactly how the range-vector [5m] is already a TimeRange wrapper rather than a Scan field. That analogy drove the design (and let me avoid churning all 35 Scan construction sites).

Changes

  • asap-ir: TimeShift { offset_ms: i64, at: Option<AtModifier> } + AtModifier::{Start, End, Timestamp(i64)}, and a QueryExpr::TimeShift { shift, child } node. Its output schema is the child's (pass-through). Offset is signed milliseconds (a negative offset -5m shifts forward); @ <ts> scales PromQL seconds → ms; @ start()/@ end() are anchor variants the runtime resolves.
  • L2 SourceSpec gains a shift field defaulted to the identity by its constructors (zero construction churn), plus a with_shift builder. The converter's scan() lifts a non-identity shift into the TimeShift wrapper; an unshifted selector stays a bare Scan.
  • PromQL front end: vs_parts now returns the shift, threaded through Inner / extract_matrix / filtered_source. A ranged selector m[5m] offset 1h shifts under its TimeRange, so the 5m window is taken at the shifted time. This unblocks the common week-over-week / anomaly-baseline pattern rate(m[5m]) - rate(m[5m] offset 1w).
  • Schema-flow, canonicalize children_mut, and the L4 bind pass all treat TimeShift as a single-child pass-through; the L4 Logical fallback wraps it unbound (it's not an aggregate).

Verified shapes

m offset 1h              → TimeShift{offset_ms:3_600_000} over Scan
m offset -5m             → TimeShift{offset_ms:-300_000} over Scan   (forward)
m @ 1609746000           → TimeShift{at:Timestamp(1_609_746_000_000)} over Scan
m @ end() offset 5m      → TimeShift{offset_ms:300_000, at:End} over Scan   (compose)
rate(m[5m] offset 1h)    → Aggregate[Rate] ∘ TimeRange{5m} ∘ TimeShift{1h} ∘ Scan

Tests

  • IR: TimeShift schema pass-through, is_identity, serde round-trip.
  • Conformance: offset (signed), @ <ts> / start() / end(), offset+@ composition, and the under-TimeRange nesting for a ranged selector.
  • Exact-tree e2e: week-over-week rate(m[5m]) - rate(m[5m] offset 1w) and up @ 1609746000.
  • The former rejection tests are flipped to passing (promql_conformance, promql_equivalence — now asserts shifted queries stay distinct from unshifted, not dropped — and the info-composition test).
  • Docs: the lowering-map table in promql.rs gains an offset/@ row.

Notes

  • Touches asap-ir / asap-l2 / both front ends' walkers. Independent of other open work.
  • Verified: cargo test --workspace — 0 failures; cargo clippy --all-targets clean; production source carries zero net-new cargo fmt drift vs main (two test files keep their pervasive compact-destructure convention).

🤖 Generated with Claude Code

…de (#40)

`m offset 1h` and `m @ 1609746000` were rejected outright — there was no
IR concept for a per-selector time shift. Neither modifier changes a
selector's schema; both only move *when* it is evaluated, so they lower
to a pass-through `TimeShift` wrapper over the selector's `Scan`
(mirroring how the range-vector `[5m]` is a `TimeRange` wrapper rather
than a Scan field).

- asap-ir: `TimeShift { offset_ms: i64, at: Option<AtModifier> }` +
  `AtModifier::{Start, End, Timestamp(i64)}`, and a
  `QueryExpr::TimeShift { shift, child }` node whose output schema is the
  child's (offset/@ never touch columns). Offset is signed ms (a negative
  offset shifts forward); `@ <ts>` scales PromQL seconds → ms.
- L2 `SourceSpec` gains a defaulted `shift` field (constructors set the
  identity, so no construction churn); the converter's `scan()` lifts a
  non-identity shift into the `TimeShift` wrapper.
- The PromQL front end's `vs_parts` now returns the shift (threaded
  through `Inner`/`extract_matrix`/`filtered_source`); a ranged selector
  `m[5m] offset 1h` shifts *under* its `TimeRange`, so the 5m window is
  taken at the shifted time. This unblocks the common week-over-week /
  baseline pattern `rate(m[5m]) - rate(m[5m] offset 1w)`.

Tests: TimeShift schema pass-through + identity/serde; conformance for
offset (signed), `@ <ts>`/`start()`/`end()`, offset+@ composition, and
the under-TimeRange nesting; exact-tree e2e pins (week-over-week + `@`);
the former rejection tests flipped (conformance, equivalence, the
info-composition test). Docs: the lowering map gains an offset/@ row.

Closes #40

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zzylol
zzylol merged commit 7f907a9 into main Jul 6, 2026
1 check passed
@zzylol
zzylol deleted the feat/40-offset-at-modifiers branch July 6, 2026 18:02
zzylol added a commit that referenced this pull request Jul 9, 2026
…113)

`main` does not compile: `cargo test -p asap-frontend-sql --test netflow`
fails with E0004, non-exhaustive patterns.

PR #106 added `QueryExpr::TimeShift` (the `offset` / `@` lowering for #40).
PR #107 added the netflow corpus, whose `visit()` matches `QueryExpr`
exhaustively, and was branched before #106 landed. Each PR was green on its
own base; neither was rebased, so the breakage only appeared once both were
on `main`.

`TimeShift` is a single-child pass-through, so it joins the existing
recurse-into-`child` arm.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PromQL offset / @ time-shift modifiers have no IR representation

1 participant