PromQL planner test helpers skip the series-identity step production runs, so ~400 Stage 1–3 tests plan over a schema production never produces #1035

Problem, by example

The PromQL query from #1011, with max_error 0.01 and failure_probability 0.001:

avg by (job) (quantile_over_time(0.5, data[5m]) / quantile_over_time(0.5, data[5m]) + quantile_over_time(0.9, data[5m]))

Lowered two ways on main (d728d05cf). Measured with a scratch crate and with stage_pipeline:

Hundreds of Stage 1–3 tests plan over the first schema, which production never produces, so they test behaviour that never runs. #1011's own example (8 groups) was measured on the test shape.

Why it happens

• The frontend (lower_promql_query_workload, crates/frontend-promql/src/lib.rs:108-133) does not add the series identity on scans.
• Production adds it in a later step, with_promql_series_identity (crates/types/src/ir/schema/schema_support.rs:40-70). That step adds the $promql_series_identity column, closes the schema and adds the key [identity, ts]; parents inherit the key (for example aggregate_schema.rs:50).
• Two production paths run that step, with different failure policies:
  • devtools lower_promql_roots (crates/devtools/src/planning_stages.rs:38-49) returns an error;
  • the facade (crates/planner/src/lib.rs:322-334) returns the raw roots, and Stage1ToStage3Pass::optimize (crates/plan-selection/src/search/pipeline.rs:340-350) adds the identity there, silently keeping the raw root when the step fails.
• The test helpers stop after the frontend:
  • test_support::lower_promql (crates/logical-asap-optimization/src/test_support.rs:21-47);
  • fixtures::lower_promql and lower_promql_root (crates/integration-tests/src/lib.rs:27-65);
  • devtools::lower_promql_with_sample_interval (crates/devtools/src/lib.rs:38);
  • direct lower_promql_query_workload plus run_stage1_to_stage3_with in crates/planner/tests/{summary_sharing,plan_stages,deployment_accuracy_model}.rs.
• The frontend is also inconsistent by itself. It already adds the identity under scalar-op and PromqlMap nodes (crates/frontend-common/src/resolve.rs:156-176), so in tests rate(x) * 2 has the production shape and rate(x) does not.
• By a grep estimate, about 350–450 Stage 1–3 tests run on the test shape. The planner_layering_example* tests already use the production path, and the frontend-shape tests (about 60, in integration-tests/tests/{aggregate,binary_op,nested,scan,schema,time_range}.rs) correctly use the raw output.

Behaviour that differs between the two shapes:

For example, promql_to_post_asap.rs:234 asserts a PromqlLabelSet heap item, which production never plans.

Proposal

Have one PromQL planning lowering, used by production and by the planner test helpers:

1. Add lower_promql_planning_roots(workload) -> Result<Vec<QueryRoot>, PromqlError> to asap-frontend-promql: the frontend lowering, then with_promql_series_identity on every operator root (and on the operator nodes a scalar root reads). It needs only asap-types, which the crate already depends on. A _with_histograms variant goes with it only if a caller needs one.
2. Call it from devtools (planning_stages.rs, replacing lower_promql_roots) and from the facade (planner/src/lib.rs).
3. Delete the identity step from Stage1ToStage3Pass::optimize, so the roots a stage receives are what the frontend handed it.
4. Pick one failure policy for the step. Recommended: return an error, as devtools does today. The facade's silent fallback plans over a shape the executor does not run. The alternative is to keep the raw root, documented.
5. Point the planner test helpers at it: test_support::lower_promql, fixtures::lower_promql / lower_promql_root, devtools::lower_promql_with_sample_interval, and the three planner/tests files. The frontend-shape tests keep the raw lower_promql_query_workload.

Adding the identity twice is harmless: a scan that already has the column is returned unchanged (schema_support.rs:44-53). So test files that already add it after the helper keep working.

Scope and acceptance

• In scope: the new function; the facade, devtools and pass changes; one failure policy; the helpers; updating test expectations that pinned the test shape (column positions, unique_keys, closed, PromqlLabelSet, CSE or reuse outcomes, prices). The estimate is dozens of tests, not yet counted by running them.
• Out of scope: SQL and MetricsQL lowering, unless they show the same gap; changing what with_promql_series_identity adds.
• Acceptance:
  • A test lowers Stage 0 review: frontends and the logical memo (hash-consed groups) #1011's query through test_support::lower_promql and asserts the quantile node has $promql_series_identity and a unique key. It fails on main today.
  • LogicalMemo::from_roots gives 7 groups for that query through every helper.
  • rg with_promql_series_identity finds it only in asap-types and in lower_promql_planning_roots (plus tests that check the step itself).
  • The facade and stage_pipeline give the same roots for the same workload.

Status and related work

• Found 2026-10-28 while measuring fix(cse): share equal nodes that return the same rows for the same input, not only keyed ones #1032 against Stage 0 review: frontends and the logical memo (hash-consed groups) #1011's example. The decision to use the single shared lowering function (above) rather than only patching the two helpers was made 2026-10-29. The failure policy is still open; the recommendation is to return an error.
• Related: Stage 0 review: frontends and the logical memo (hash-consed groups) #1011 (Stage 0 review; checklist 2 asks for one frontend entry-point shape), fix(cse): share equal nodes that return the same rows for the same input, not only keyed ones #1032 (CSE legality, whose only effect on the test path is this gap), [Refactor] Repo-wide naming survey: overloaded and misleading names #948 (naming survey).

🤖 Generated with Claude Code