Skip to content

Remove test-only query planning and unused backend APIs and dependencies - #717

Merged
zzylol merged 2 commits into
mainfrom
refactor/production-query-plan-tests
Sep 13, 2026
Merged

zzylol merged 2 commits into
mainfrom
refactor/production-query-plan-tests

Conversation

@zzylol

@zzylol zzylol commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Why

Tests should exercise the installed QueryPlan path used in production. Unused APIs, compatibility wrappers, and dependencies obscure that boundary and add build work.

Addresses the test-only planning, unused public items, types_v2, and dependency cleanup portions of #703.

What

  • Remove data-plane PromQL replanning and the test-only instant/range execution branches. Runtime tests install explicit QueryPlans.
  • Delete unused APIs and restrict unit-test helpers to test builds. Preserve production callers of mapped query-plan compilation and hot-reload lifecycle operations.
  • Merge types_v2 into types, replace BindingName with String, and remove unused helpers and the module-wide dead-code allowance.
  • Remove six unused direct dependencies and share reqwest 0.12 across both planes. Keep tokio-stream's required net feature explicit in data_plane.

How

Fixtures choose materializations and readouts explicitly, install a validated physical plan, and execute through the production engine. Tests supply complete panes for successful reads and verify that missing plans, unknown queries, missing samples, and partial coverage fail closed. The observed-family planning bridge and its cost-model adapter are removed.

Before this PR

A test with an HLL sketch but no installed plan could answer count(unique_users_per_min), while production rejected the same request. Range tests could accept incomplete warm coverage. Both reqwest versions and bundled SQLite appeared in the dependency graph.

After this PR

The unplanned query is rejected in both builds. An explicitly installed binding serves the stored HLL estimate through the production path; incomplete range coverage returns a capability miss for archive routing. The lockfile contains one reqwest version and no SQLite package.

Evidence

The new no-plan regression failed against the original implementation and passes after the change. Installed-plan tests retain HLL union, KLL full/delta, CMS, exact aggregation, and bound-policy isolation coverage. The lockfile loses 53 packages without adding or upgrading packages. Screenshots and runtime performance measurements are not applicable.

Verification

Using Rust 1.98.0:

  • Unit tests: data_plane — 1,174 passed; control_plane — 737 passed.
  • End-to-end tests: e2e_controller_plans_and_backend_serves — 12 passed.
  • Other checks: cargo check --workspace --all-targets --locked --offline, formatting, and git diff --check passed.

New tests verify that stored sketches alone cannot authorize a query, installed plans reject unknown identities and missing samples, and partial warm coverage is rejected with or without an attached archive client.

Architectural decisions

Planning stays in the control plane; serving consumes installed bindings. Test setup does not infer families or parameters from query text. Production-used compile_bound_*_mapped, discard_staged, retire_drained, and materialization_statuses remain available to their callers.

Limitations and follow-up

This addresses part of #703, not the entire issue. uuid and zstd remain as transitive dependencies. No production deployment behavior or accuracy formulas are changed; existing test expectations that depended on test-only replanning are replaced by installed-plan coverage.

Human review — do not complete with an agent

  • The MVP boundary is correct.
  • New conceptual layers or public interfaces are necessary.
  • The before/after description matches the intended product behavior.
  • Human reviewer:
  • Decision and rationale:

@zzylol
zzylol merged commit aba025b into main Sep 13, 2026
1 check failed
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.

1 participant