Skip to content

refactor(query-engine): make native DAG self-contained at execution #754

Description

@milindsrivastava1997

Problem

Issue #729 introduced a native range-query DAG, but the plan is currently only partly executable. After compilation, NativePlanRuntime still retains &RangeQueryExecutionContext and uses it as the semantic authority for several nodes. A plan can therefore describe one operation while runtime executes behavior derived from duplicated context fields.

This was identified during PR #745 review. The grouped-topk regression discussed there is already fixed; this issue addresses the separate plan/runtime ownership design gap.

Decision

Keep RangeQueryExecutionContext as a compilation-only input.

After QueryPlan::compile_range succeeds, query semantics must live exclusively in QueryPlan. NativePlanRuntime may retain execution resources such as &SimpleEngine, the store, and execution-local caches, but must not retain or consult RangeQueryExecutionContext.

Target flow:

PromQL request + configured precomputes
        -> RangeQueryExecutionContext  (compile-time only)
        -> QueryPlan                   (complete semantic recipe)
        -> NativePlanRuntime { engine, cache }  (resources only)

Current plan/context duplication

  • StoreRead carries a query and strategy, but runtime ignores strategy, compares the node query against context.base.store_plan, and fetches both value/key reads through context.
  • ComposeWindows declares output timestamps, lookback, window size, and bucket step, but runtime ignores those fields.
  • Estimate carries statistic and query_kwargs, but runtime calls estimate_range_query(context, ...); range windows, aggregation metadata, labels, and bounds come from context instead.
  • LimitTopK carries k and grouping labels, but runtime derives row-label order from context.
  • Format is substantially self-contained already.

This makes the current DAG descriptive rather than the single source of executable semantics.

Scope

  1. Lower every range-query execution semantic needed after compilation into explicit plan-node data or compact plan-owned specification types.
  2. Remove RangeQueryExecutionContext from NativePlanRuntime.
  3. Make each node execute according to its own declared parameters.
  4. Make StoreRead independently identify and perform its requested read strategy; do not select data by comparing against context-owned query fields.
  5. Resolve the ComposeWindows mismatch deliberately:
    • either make it genuinely compose windows from its declared fields; or
    • rename it to an honest indexing/normalization operation and remove unused semantic fields.
  6. Move top-k row-label ordering into LimitTopK.
  7. Preserve current public behavior and error/fallback contract:
    • Ok(None) is an unsupported native capability miss and may fall back;
    • Err(QueryExecutionError) is an accepted native execution failure and must remain local.

A likely shape is a plan-owned RangeEstimateSpec containing value/key window specifications, aggregation metadata needed by merging/key resolution, output labels, statistic, kwargs, and per-step bounds inputs. Do not copy the existing context wholesale into a node: extract cohesive plan-owned specs with explicit invariants.

Non-goals

Acceptance criteria

  • NativePlanRuntime has no RangeQueryExecutionContext field or argument.
  • Mutating a compile-time context after plan construction cannot alter plan execution; ideally this is enforced structurally because execution accepts only plan + runtime resources.
  • Each node either consumes all fields that define its semantics or those fields are removed/renamed.
  • A plan with distinct value/key reads executes the specified node reads without context identity comparisons.
  • Existing DAG-only E2E behavior coverage remains green, including tumbling/sliding, separate keys, DeltaSet replay, top-k ties/grouping, capability misses, malformed-plan handling, and local HTTP no-fallback behavior.
  • Existing legacy-vs-DAG E2E characterization coverage remains green under native_query_legacy_test_support.
  • The PromQL Docker compliance matrix has no regression relative to main.

Suggested tests

Prefer public E2E and plan-execution behavior tests over isolated helper tests:

  • Compile a range plan, execute it with a runtime that has no query context, and compare public range-query results to the current DAG baseline.
  • Exercise two different read/window specifications in one plan shape (value plus separate keys) to prove each node uses plan-owned semantics.
  • Ensure grouped top-k uses plan-owned k, grouping labels, and row-label order.
  • Retain malformed-plan validation and failing-store/no-fallback tests; they ensure plan/runtime errors remain explicit.

References

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions