feat: shared Layer 2→3 lowering pass — both PromQL and SQL emit Aggregate(AggFunc) - #118
Merged
Merged
Conversation
Both parsers now emit Layer 2 relational operators (Aggregate + AggFunc), and a shared lowering pass (algebra/lower.rs) converts to Layer 3 sketch algebra (SketchAgg + AggIntent). Layer compliance: - Layer 1-2 (parsers): zero AggIntent in code — only Aggregate + AggFunc - Layer 2→3 (lower.rs): shared lower_to_sketch_algebra() for both languages - Layer 3 (expr.rs): AggIntent, zero physical sketch names in code - Layer 4 (optimizer.rs): matches AggIntent, zero SketchType references - Layer 5 (physical.rs): resolve() maps AggIntent → SketchType + SketchParams Changes: - promql.rs: walk_call_to_op returns AggFunc (not AggIntent); build_qe_sketched → build_qe_aggregate emitting Aggregate + Window nodes - lower.rs: new shared lowering pass (18 tests) — walks all 25 QueryExpr variants, converts single-agg Aggregate → SketchAgg where sketchable - mod.rs: parse_query_expr calls lower_to_sketch_algebra after parsing 301 tests pass (283 + 18 new lowering tests). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced May 9, 2026
zzylol
added a commit
that referenced
this pull request
May 9, 2026
Three small follow-ons after PR #365 (asap-gorilla symlink delete) + PR #363 (gorillaprocessor / per-sketch cmd dirs delete) made the service rebuild flow brittle: ## (1) `Dockerfile.backend` — repoint asap-gorilla path-dep The backend's `asap-query-engine/Cargo.toml` path-dep was updated to `asap-gorilla = { path = "../../ASAPCollector/asap-gorilla-rust" }` in PR #118 (backend repo) when the symlink was being removed, but the matching Dockerfile still copied to `ASAPCollector/asap-gorilla` (the now-deleted symlink path). Result: cargo couldn't find the crate inside the build container; backend image rebuild failed. Repoints the COPY: - `--from=asap-gorilla → ASAPCollector/asap-gorilla` → - `--from=asap-gorilla-rust → ASAPCollector/asap-gorilla-rust` Caller has to pass the matching `--build-context asap-gorilla-rust=/path/to/asap-gorilla-rust` (mirrors the existing `asap-precompute-rs` build-context pattern). ## (2) `cmd/asap-otel/builder-config.yaml` — add asap-gorilla-go replace `gorillas3processor`'s own go.mod has `replace github.com/ProjectASAP/asap-gorilla-go => ../../../asap-gorilla-go`, but OCB does NOT transitively forward processor-level replaces into the binary's generated `cmd/asap-otel/go.mod`. Result: OCB tried to fetch `asap-gorilla-go` from github.com (private/non-existent public path), build failed with "Repository not found". Adds the replace at the binary level, mirroring the existing `sketchlib-go` and `asap-precompute-go` redirects. Path math documented inline (binary go.mod is 3 dirs deep under repo root). ## (3) `mvp-no-resource-limits.yml` — opt-in override The MVP demo's `mvp-multi-stage.yml` hardcodes `cpus: 1.5` and `memory: 1536M` on agents (and similar caps on gateway / Thanos services). At any meaningful cardinality (≥2k aggregate series), the agent's 5-sketch fan-out + windowState buffer eats 7+ GiB of RSS and gets OOM-killed (SIGKILL/137). Each restart flushes a 1-ms-wide TSDB block — the cold-path-through-thanos returns present-but-degenerate data. Override lifts both `cpus` and `memory` (the whole `limits:` block) for: agent-a, agent-b, gateway, prometheus-b0, thanos-store-gateway, thanos-query, thanos-compact. Apply by appending `-f deploy/docker-compose/mvp-no-resource-limits.yml` to the existing compose chain. Default behaviour unchanged for paper-eval reproducibility. Verified with `docker compose ... config | python3 -c '...'`: all 7 services show `limits=None`. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #117.
Summary
Both parsers now follow the same 5-layer architecture:
Aggregate { AggFunc }+Window(Layer 2), notSketchAgg { AggIntent }(Layer 3)algebra/lower.rs: shared lowering passlower_to_sketch_algebra()convertsAggregate → SketchAggfor both languagesparse_query_expr()callslower_to_sketch_algebra()after parsing for both SQL and PromQLLayer compliance:
AggIntentin code, 23AggFuncrefsAggIntentin code (already clean)Test plan
🤖 Generated with Claude Code