feat(querier): expand resolves_to via the alias map (RFC0002.9) - #154
Conversation
Wire the RFC 0001 §6.7 alias map into the DSL compile path so `resolves_to(n)` matches the whole per-tenant equivalence class of `n`, not just `n` itself. `Querier::run_query` now takes an `&ourios_core::alias::AliasMap` (an in-memory projection the caller injects; no on-disk loading — that is the RFC 0005 storage split). The class is resolved per-tenant at compile time and the call compiles to `template_id IN (class)`. A singleton class (no alias on `n`) is `template_id IN (n)`, behaviourally identical to a bare `template_id == n`, so non-aliased queries are unchanged. This is the intended, maintainer-approved contract change for RFC0002.9: from the prior base-member semantics (`resolves_to(n)` == `template_id == n`) to true alias-set expansion. The RFC0002.9 scenario flips from an ignored stub to a real cross-alias integration test against a genuine RFC 0005 store: with B aliased to A under tenant T, `resolves_to(A)` matches both A and B (excludes a control C), bare `template_id == A` matches only A, the expansion is symmetric, and the alias does not leak to a second tenant (§3.7 isolation). Hazard CLAUDE.md §4.6: no datafusion/arrow/SQL type or string crosses a public boundary — `AliasMap` is an ourios-core type; the `in_list` is internal to the compiler. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 13 minutes and 5 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
Wires the RFC 0001 per-tenant AliasMap into the querier’s DSL compilation so resolves_to(n) expands to the full alias equivalence class, and replaces the RFC0002.9 ignored stub with a real integration test covering cross-alias behavior.
Changes:
- Extend
Querier::run_queryto accept an injected&ourios_core::alias::AliasMapand thread it into compilation. - Resolve
resolves_to(n)at compile time intotemplate_id IN (alias_class)via an eagerly collected alias-class map on thePlan. - Flip RFC0002.9 from an ignored
unimplemented!()to a real async integration test validating alias expansion.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/ourios-querier/src/lib.rs | Adds alias_map parameter to run_query and forwards it into the compiler. |
| crates/ourios-querier/src/compile.rs | Collects alias classes at compile time and compiles resolves_to into an IN (...) filter. |
| crates/ourios-querier/tests/rfc0002_dsl.rs | Threads an empty alias map through existing tests and adds an RFC0002.9 integration test for alias expansion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…-tenant test Review: the no_aliases helper + run_query doc now say resolves_to(n) compiles to a singleton template_id IN (n) (behaviorally == n), not == n; the RFC0002.9 cross-tenant case now reuses the same populated AliasMap for T2 (proving per-tenant scope) instead of a separate empty map. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review: RFC0002.9 is "Template primitives compile" — the cross-alias test now also asserts lossy == true and confidence < 0.7 filter (added a lossy row retaining its body per §3.3 and a low-confidence row), alongside template_id == n and resolves_to expansion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e blocker Review: RFC 0002 status note now credits #143 (the spec PR) alongside #144-#154; RFC 0007 §8 + the alternatives no longer say parser integration is blocked on the RFC 0002 §3 branch decision (resolved, Branch B) — the contradiction with the green status note is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
RFC 0002 (query DSL) is 11/11 green (#143-#154) and RFC 0007 (querier execution frontend) RFC0007.1-.5 are all live + passing; the prove-thesis gate is cleared. RFC 0001 deliberately stays specified (its miner criteria .5/.6/.8/.9/.10/.11 + hazards/invariants remain red-gate stubs). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
SLICE B (final slice) of the RFC 0001 alias-index write path: wire the
in-memory alias map into the querier so
resolves_to(n)expands to thewhole RFC 0001 §6.7 equivalence class of
n, flipping the last RFC 0002criterion RFC0002.9 from an ignored stub to a real cross-alias test.
Builds on SLICE A (#153 —
ourios-coreAliasMap+ auditedassert/retract +
resolves).What changed
Alias-map threading.
Querier::run_querynow takes an&ourios_core::alias::AliasMapalongside the existingtenant/now/default-window:
The map is an in-memory projection injected by the caller — no
on-disk loading is added here; the physical map artifact (serialization,
snapshot cadence) is the RFC 0005 storage split (sibling to #147),
flagged not built.
in_listexpansion.compile::compileresolves everyresolves_to(n)in the predicate against the tenant's map once, atcompile time (
collect_alias_classes), and stores the sorted class onthe
Plan.Call::ResolvesTo(n)then compiles totemplate_id IN (class)(a DataFusionin_listover the sorted ids). A singleton class(no alias on
n) istemplate_id IN (n)— behaviourally identical tothe prior
template_id == n, so non-aliased queries are unchanged.Contract change (intended, maintainer-approved). RFC0002.9 moves
from base-member semantics (
resolves_to(n)==template_id == n)to true alias-set expansion. The criterion's §5 description already
specifies the expansion, so no RFC doc edit was needed; the gate moves
from an
#[ignore]'dunimplemented!()stub to a real integration test.RFC0002.9 — real cross-alias test
Against a genuine RFC 0005 store (same fixture/store-builder the other
querier integration tests use):
Twith rows for templatesA,B, and a controlC;AliasMapbuilt via the ourios-core operator API assertingBaliases
AunderT;resolves_to(A)matches bothAandB, excludesC;template_id == Amatches onlyA(the distinction is the point);resolves_to(B)is the same class (symmetry);Tdoes not leak to a second tenantT2(empty map):
resolves_to(A)there matches onlyA's row (§3.7isolation).
Hazards
Hazard
CLAUDE.md§4.6 (no DataFusion/arrow/SQL leakage): preserved.AliasMapis an ourios-core type; thein_listis internal to thecompiler. The public
run_querysignature names only Ourios + stdtypes. §3.7 multi-tenancy: alias resolution is strictly per-tenant and
covered by the cross-tenant assertion.
Verification (local)
cargo test --all-features— 534 passed; 0 failed; 41 ignored(RFC0002.9 + all RFC0002.x green; RFC0001.12–.16 green; whole
workspace green)
cargo fmt --all --check— cleancargo clippy --all-targets --all-features -- -D warnings— cleanFollow-up (flagged, not built)
How a querier process obtains the
AliasMapin production (load /snapshot / refresh of the physical map artifact) is the RFC 0005
physical-storage split, sibling to #147. This PR keeps the map an
injected in-memory projection.
🤖 Generated with Claude Code