feat(query-engine): add native instant binary-expr evaluator, staged ahead of cutover by milindsrivastava1997 · Pull Request #572 · ProjectASAP/ASAPQuery

milindsrivastava1997 · 2026-08-21T17:32:44Z

Summary

New native (non-DataFusion) implementation of PromQL binary-arithmetic instant queries: a recursive arm evaluator over Vec<InstantVectorElement>, plus vector-vector and scalar combiners lifted from the existing range-binary path (handle_binary_expr_range_promql).
Not wired into production dispatch yet — handle_query_promql still calls the DataFusion path unchanged. The new path is exposed via handle_query_promql_native/handle_binary_expr_promql_native purely so it can be equivalence-tested against DataFusion before the cutover (Stage 3).
One accepted, deliberately loud behavior change baked in now rather than discovered later: an arm with zero current precomputed data falls back to Prometheus (None) instead of DataFusion's silent empty-result behavior, with a warn! making it observable. See asap-query-engine: convert binary PromQL instant queries from DataFusion to native execution #567 for the full writeup.

Stage 2 of #567 (converting binary-arithmetic PromQL instant queries from DataFusion to native execution). Stacked on #570 (Stage 1, sliding-bucket merge fix) — this PR's base branch is that one, not main.

Test plan

New native_binary_instant_tests.rs: all ops (+ - * / %) old-vs-new equivalence, scalar both-orderings, nested binary (a+b)*c, empty-arm fallback behavior, unsupported-arm parity, dual-population leaf (keys_query), and a dedicated ^ test (DataFusion's ^ support is separately broken today — maps to Operator::BitwiseXor "as a proxy" per its own comment and produces no result at all; native computes it correctly via f64::powf, nothing to compare against on the old side for that one operator)
Full cargo test --lib (539 tests) green
cargo clippy --lib --tests -- -D warnings clean

… fix stale dead-code comment handle_binary_expr_range_promql's vector-vector join matched purely on positional KeyByLabelValues equality (rhs labels discarded), unlike the instant-query combine_vector_vector fixed earlier in this stack (#572) to reject a join between arms grouped by different label sets. Two arms grouped by disjoint labels (e.g. (host) vs (region)) whose values happened to coincide could silently join into a wrong-but-plausible result across the whole range. Now rejects the join (returns None) when lhs_labels != rhs_labels, mirroring the instant-query guard. Extends create_range_engine_two_metrics (range_query_arithmetic_tests.rs) to take per-metric grouping labels, and adds a regression test proving the divergence. Also corrects execute_logical_plan's doc comment: it claimed to be "part of the still-exercised DataFusion path (see its dedicated tests)", but it has zero callers anywhere in the repo, including tests -- unlike its sibling execute_plan, which genuinely is still called by DataFusion-path tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ahead of cutover Builds a native (non-DataFusion) implementation of PromQL binary- arithmetic instant queries: a recursive arm evaluator over Vec<InstantVectorElement>, plus vector-vector and scalar combiners lifted from the existing range-binary path. Not yet wired into production dispatch (handle_query_promql still calls the DataFusion path) — exposed via handle_query_promql_native for equivalence testing against the DataFusion path ahead of the Stage 3 cutover. One accepted, deliberately loud behavior change: an arm with zero current precomputed data now falls back to Prometheus (matching "not acceleratable" semantics) instead of DataFusion's silent empty-result behavior, with a warn! so it's observable. Part of #567 Stage 2. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

… combiner combine_vector_vector_native joined two arms purely by positional KeyByLabelValues equality (a bare Vec<String> of values, no label names attached). DataFusion's build_binary_vector_plan joins on named columns instead, so it fails to resolve (-> None) whenever the two arms don't share the same label set. Native had no equivalent check, so it could silently fabricate a joined result whenever two differently-grouped arms' values happened to coincide (e.g. `sum(a) by (host) + sum(b) by (region)` with a host value equal to a region value). combine_vector_vector_native now takes both arms' label-name lists and returns None on a mismatch, matching DataFusion's failed-join behavior. Adds four regression tests proving the divergence (three were red before this fix) plus a same-label-set control case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…af arms evaluate_arm_native's leaf branch hardcoded execute_query_pipeline(&ctx, false, false), so a topk arm inside a binary expression (e.g. `topk(10, metric) + 0`) never got truncated to k or metric-name-prefixed — both flags are self-gated on statistic == Topk / a "k" kwarg being present (see execute_query_pipeline's doc comment), so passing (true, true) unconditionally is a no-op for non-topk arms, matching what the main non-binary instant-query path already does. Adds a regression test proving the divergence: topk(10, ...) + 0 returned all 15 unformatted rows before this fix, now correctly truncates to 10 with the metric-name prefix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

… fix stale dead-code comment handle_binary_expr_range_promql's vector-vector join matched purely on positional KeyByLabelValues equality (rhs labels discarded), unlike the instant-query combine_vector_vector fixed earlier in this stack (#572) to reject a join between arms grouped by different label sets. Two arms grouped by disjoint labels (e.g. (host) vs (region)) whose values happened to coincide could silently join into a wrong-but-plausible result across the whole range. Now rejects the join (returns None) when lhs_labels != rhs_labels, mirroring the instant-query guard. Extends create_range_engine_two_metrics (range_query_arithmetic_tests.rs) to take per-metric grouping labels, and adds a regression test proving the divergence. Also corrects execute_logical_plan's doc comment: it claimed to be "part of the still-exercised DataFusion path (see its dedicated tests)", but it has zero callers anywhere in the repo, including tests -- unlike its sibling execute_plan, which genuinely is still called by DataFusion-path tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…tive execution (#577) * feat(query-engine): cut over PromQL binary-expr instant queries to native execution Rewires handle_binary_expr_promql to call the native evaluator/ combiners (built in the prior stage) instead of building a DataFusion plan. Deletes the old DataFusion-based handle_binary_expr_promql, build_arm_logical_plan, and the tokio::task::block_in_place(... block_on(...)) wrapper it needed (native execution is synchronous). Renames evaluate_arm_native/combine_vector_vector_native/ combine_scalar_native -> evaluate_binary_arm/combine_vector_vector/ combine_scalar now that native is the only implementation, and drops the now-redundant handle_binary_expr_promql_native/ handle_query_promql_native test-only wrappers. This was the only production code path still reachable through DataFusion (asap-query-engine's SimpleEngine now serves every query shape through the same native fetch/merge pipeline). DataFusion's CustomQueryPlanner/PrecomputedSummaryReadExec/SummaryMergeMultipleExec/ build_binary_vector_plan/build_scalar_plan and the datafusion dependency itself are left in place, per #567's scope — still exercised by their own dedicated tests and the unwired execute_plan prototype, just no longer reachable from production. Closes #567 Stage 3 (final stage). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(query-engine): address code-review cleanup findings on PR #567 Stage 3 cutover - move plan_execution_arithmetic_tests.rs out of tests/datafusion/ since it now exercises the native binary-expr path, not DataFusion - fix warn! in evaluate_binary_arm that mislabeled any execute_query_pipeline error as "produced no results" - mark orphaned execute_logical_plan #[allow(dead_code)] and update its doc comment, matching its sibling execute_plan - avoid an unnecessary Vec<String> clone in combine_vector_vector - mark design-252 doc as superseded by the native cutover in #567 * fixup(query-engine): update topk-in-binary-expr test for post-cutover API handle_query_promql_native was removed by the #567 Stage 3 cutover (native is now the only path, folded into handle_query_promql) — this commit was squashed into the branch during rebase onto the cutover. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(query-engine): reject cross-label-set joins in range binary-expr; fix stale dead-code comment handle_binary_expr_range_promql's vector-vector join matched purely on positional KeyByLabelValues equality (rhs labels discarded), unlike the instant-query combine_vector_vector fixed earlier in this stack (#572) to reject a join between arms grouped by different label sets. Two arms grouped by disjoint labels (e.g. (host) vs (region)) whose values happened to coincide could silently join into a wrong-but-plausible result across the whole range. Now rejects the join (returns None) when lhs_labels != rhs_labels, mirroring the instant-query guard. Extends create_range_engine_two_metrics (range_query_arithmetic_tests.rs) to take per-metric grouping labels, and adds a regression test proving the divergence. Also corrects execute_logical_plan's doc comment: it claimed to be "part of the still-exercised DataFusion path (see its dedicated tests)", but it has zero callers anywhere in the repo, including tests -- unlike its sibling execute_plan, which genuinely is still called by DataFusion-path tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

milindsrivastava1997 mentioned this pull request Aug 21, 2026

feat(query-engine): cut over PromQL binary-expr instant queries to native execution #573

Closed

4 tasks

milindsrivastava1997 force-pushed the 567-2-native-instant-binary-evaluator branch from 22d0f97 to ce9dc1f Compare August 23, 2026 02:02

milindsrivastava1997 force-pushed the 567-1-sliding-bucket-merge-fix branch from 506489e to 8e768fa Compare August 23, 2026 02:46

milindsrivastava1997 force-pushed the 567-2-native-instant-binary-evaluator branch from defd004 to d86abd6 Compare August 23, 2026 02:46

Base automatically changed from 567-1-sliding-bucket-merge-fix to main August 23, 2026 02:49

milindsrivastava1997 and others added 3 commits August 22, 2026 22:52

milindsrivastava1997 force-pushed the 567-2-native-instant-binary-evaluator branch from d86abd6 to f814b6c Compare August 23, 2026 02:53

milindsrivastava1997 merged commit b83918a into main Aug 23, 2026
5 checks passed

milindsrivastava1997 deleted the 567-2-native-instant-binary-evaluator branch August 23, 2026 02:54

milindsrivastava1997 mentioned this pull request Aug 23, 2026

feat(query-engine): cut over PromQL binary-expr instant queries to native execution #577

Merged

6 tasks

Navigation Menu

Uh oh!

feat(query-engine): add native instant binary-expr evaluator, staged ahead of cutover - #572

feat(query-engine): add native instant binary-expr evaluator, staged ahead of cutover#572
milindsrivastava1997 merged 3 commits into
mainfrom
567-2-native-instant-binary-evaluator

milindsrivastava1997 commented Aug 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

milindsrivastava1997 commented Aug 21, 2026

Summary

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant