asap-query-engine: convert binary PromQL instant queries from DataFusion to native execution

[milindsrivastava1997]

Context

asap-query-engine's SimpleEngine currently serves PromQL binary-arithmetic
instant queries (vector-vector and vector-scalar) through a dedicated
DataFusion path (handle_binary_expr_promql → build_arm_logical_plan →
build_binary_vector_plan/build_scalar_plan → execute_logical_plan), while
every other live query shape (plain vector-selector instant queries, both
PromQL range-query shapes, all of SQL, all of Elastic DSL) is served by the
native/direct path (execute_query_pipeline → execute_and_merge_store_queries
→ Store::query_precomputed_output[_exact] + hand-written merge).

This is the only place DataFusion is reachable in production at all. Having
one query shape run through a structurally different engine means the two
implementations of "fetch precomputed outputs and merge them" have to be kept
behaviorally equivalent by hand — and they already aren't (see design
decisions below).

Full audit this issue was scoped from: .design_docs/query-engine-fetch-merge-audit.md.

Design decisions (resolved via grilling session, 2026-08-21)

• Sliding-bucket merge bug is real and must be fixed first, its own stage.
  execute_and_merge_store_queries (simple_engine/mod.rs:551-567) takes the
  first bucket per key for Sliding-window queries and discards the rest
  (warn-only). SummaryMergeMultipleExec (DataFusion) merges all of them
  correctly. Binary-expr queries are currently the one shape immune to this,
  because they're the one shape still on DataFusion — cutting them over
  as-is would silently inherit the bug. DataFusion's merge-all behavior is
  the correct one; native gets fixed to match, not the other way round.
  Expected-count-per-key is hardcoded to 1 for now (ponytail: comment
  pointing at Sliding window execution: support multi-window merge/subtract in query engine #554, which will make >1 legitimate — don't block on it).
  Done: fix(query-engine): merge all sliding-window buckets per key instead of taking first #570.

• Label-order join "bug" investigated, no live bug found — dropped.
  Originally scoped as its own stage: the vector-vector join
  (handle_binary_expr_range_promql) keys a HashMap by raw
  KeyByLabelValues (positional Vec<String>), and the concern was that two
  metrics sharing a label set could declare it in different order, silently
  breaking the join. Turns out this can't happen: KeyByLabelNames::new()
  (promql_utilities/data_model/key_by_label_names.rs:10-18)
  unconditionally sorts label names, and every source of label-name ordering
  reachable from a PromQL binary expression — PromQLSchema.get_labels(),
  and the by(...)/without(...) clause derivation
  (get_spatial_aggregation_output_labels) — routes through it. Confirmed
  with an isolated scratch test (two metrics, deliberately reversed
  declaration order, both come back canonically sorted).
  One real bypass exists — asap-planner-rs/src/planner/elastic_dsl.rs:126-129
  constructs KeyByLabelNames via struct literal, unsorted, from the
  Elastic DSL query's own group_by clause order — but it's unreachable
  from this join: Elastic DSL has no binary-expr concept, and a SimpleEngine
  instance is permanently locked to one QueryLanguage at construction
  (simple_engine/mod.rs:1040-1044), so an engine holding Elastic-planned
  (possibly unsorted) AggregationConfigs never routes through
  handle_binary_expr_promql/handle_binary_expr_range_promql regardless of
  input. See Elastic DSL's unsorted KeyByLabelNames construction would break if binary arm support is ever added #571 for the forward-looking risk if Elastic ever gains binary
  arm support.

• Nested binary arms ((a+b)*c) must be preserved in the native instant
  evaluator. DataFusion's build_arm_logical_plan recurses on nested
  Binary arms today (promql.rs:379-387); the range-binary path being
  borrowed from does not (tracked separately as promql.rs: range queries don't accelerate nested binary arithmetic #516, out of scope here).
  The native instant evaluator needs its own recursion so this doesn't
  regress.

• DataFusion removal from the crate is out of scope for asap-query-engine: convert binary PromQL instant queries from DataFusion to native execution #567.
  CustomQueryPlanner/PrecomputedSummaryReadExec/SummaryMergeMultipleExec/
  build_binary_vector_plan/build_scalar_plan/the datafusion dependency,
  and the unwired execute_plan prototype (mod.rs:717) that still uses them
  in tests, all stay as-is. asap-query-engine: convert binary PromQL instant queries from DataFusion to native execution #567 only stops production from calling
  DataFusion. Deletion is a deliberate follow-up once the native path has
  proven itself, not bundled into this cutover.

• TDD, red tests first, every stage. All tests land as a permanent,
  committed suite (including the old-DataFusion-vs-new-native equivalence
  checks) — not a throwaway local proof. This supersedes the usual
  "local-only equivalence check, don't commit it" default for critical
  refactors, specifically because this work is fixing real bugs and adding
  new permanent behavior, not just re-deriving something that already existed.

• Stage boundaries are commit/pause points. Each stage lands
  independently (build/test/clippy green) and stops for manual commit —
  no auto-continuing through the plan.

Staged plan

Stage 1 — fix the sliding-bucket merge bug (simple_engine/mod.rs:551-567)
Merge all buckets per key instead of taking the first; warn (not drop) when
count ≠ expected(1). Done: #570.

Stage 2 — build the native instant binary evaluator

1. Leaf resolution: swap ctx.to_logical_plan() for
   execute_query_pipeline(&ctx, false, false) (promql.rs:389-391) →
   Vec<InstantVectorElement> instead of LogicalPlan.
2. Native recursive arm evaluator mirroring build_arm_logical_plan
   (promql.rs:368-394): leaf → pipeline result; nested Binary → recurse
   both sides then combine; Paren → recurse; NumberLiteral → None.
3. Native vector-vector combiner, lifted from the range binary path
   (promql.rs:585-620) — positional KeyByLabelValues join is fine as-is
   (see label-order design decision above).
4. Native scalar combiner, lifted from the range binary scalar branch
   (promql.rs:569-582); apply_range_binary_op (promql.rs:457-472) is
   reusable as-is (maybe rename, "range" no longer accurate).

Stage 3 — cutover
Rewire handle_binary_expr_promql (promql.rs:400-454): keep
detect_scalar_arm dispatch, swap the DataFusion-plan-building calls for the
new native evaluator/combiners, delete the
tokio::task::block_in_place(...block_on(execute_logical_plan(...)))
wrapper (native execution is synchronous, no async/blocking dance needed).

Tests (TDD, red before implementation, all permanent)

Stage 1 — src/tests/native_pipeline_merge_tests.rs (done, #570):

1. sliding_single_bucket_returns_its_value — 1 bucket, baseline unchanged.
2. sliding_two_buckets_for_same_key_are_merged_not_dropped — 2 buckets → merged, not first-only. (red test for today's bug)
3. sliding_bucket_count_mismatch_still_returns_merged_result — count ≠ 1 still merges + warns, doesn't drop.
4. tumbling_multi_bucket_merge_unaffected_by_sliding_fix — Tumbling branch (already correct) untouched.

Stage 2 — src/tests/native_binary_instant_tests.rs:
5. native_vector_vector_all_ops_match_datafusion — table-driven old-vs-new equivalence across + - * / % ^.
6. native_vector_scalar_both_orderings_match_datafusion — 2*foo and foo*2 both match DataFusion.
7. native_nested_binary_matches_datafusion — (foo+bar)*baz matches DataFusion. (nested-arm parity)
8. native_binary_expr_no_data_falls_back_to_none — one arm's store query errors → None (Prometheus fallback), no panic.
9. native_binary_expr_unsupported_arm_returns_none — non-acceleratable arm → None, unchanged from today.
10. native_vector_vector_dual_population_matches_datafusion — one arm is dual-population (separate keys_query), old vs new match.

Stage 3 — extends native_binary_instant_tests.rs:
11. sliding_window_binary_expr_end_to_end_merges_correctly — through real handle_query_promql: Sliding-window binary-expr query with >1 bucket/key returns the fully-merged value.
12. native_binary_expr_works_on_current_thread_runtime — run under #[tokio::test] (single-threaded, not multi_thread) — only passes once block_in_place is actually gone.
13. Existing dispatch_arithmetic_tests.rs suite re-run unmodified as a black-box regression guard.

[milindsrivastava1997]

Considered whether the range-query pipeline could just be reused directly for
instant queries (both usual and binary arms) instead of writing the native
evaluator/combiners in the steps above. It's blocked in a few concrete ways:

start == end is explicitly rejected. validate_range_query_params
(mod.rs:1292) has if start >= end { return Err("start must be before end") }.
An instant query is exactly start == end, so you can't call the range
pipeline with the true instant point at all — you'd have to fake a tiny
non-zero width, which changes what's actually being evaluated (last point of
a synthetic small range, not "at this instant"), and step also has to be a
positive multiple of tumbling_window_ms (:1298), so you can't even pick an
arbitrary trivial step.

execute_range_query_pipeline never looks at keys_query at all. Grep
for keys_query across mod.rs turns up exactly one function body that
touches it — execute_and_merge_store_queries (:591), the native instant
path. The range pipeline only ever fetches context.base.store_plan.values_query
(:1431); there's no code path in it that fetches or merges a separate keys
aggregation. So any dual-population metric (MultipleSum/MultipleMinMax/
HydraKLL paired with a DeltaSetAggregator keys aggregation) would silently
drop the keys side if routed through range. This isn't a missing detail, it's
a missing feature.

Sliding windows are structurally incompatible with the range pipeline,
same issue as the sliding-window doc flags elsewhere: it forces
is_exact_query = false and grid-steps by tumbling_window_ms
unconditionally (promql.rs:1141-1144), which is exactly backwards for
Sliding aggregations (single exact-window fetch, no grid). Routing
instant-Sliding queries through range would hit the same mismatch already
flagged in the sliding-window doc.