Skip to content

feat(planner): classify ASAPQuery rules into existing strategies - #333

Merged
zzylol merged 1 commit into
mainfrom
feat/asapquery-rule-coverage
Sep 3, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/asapquery-rule-coverage

Conversation

@zzylol

@zzylol zzylol commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Why

ASAPQuery contains useful planner and optimizer rules, but copying PromQL syntax patterns into a parallel ASAPPlanner strategy would duplicate ownership and make coverage language-specific. The rules need to be audited by semantic decision and assigned to the ASAPPlanner component that already owns that decision.

What

  • Audits ASAPQuery planner and optimizer rule families at commit 2586400b3b0436a5414c901ebce07065d20b5223.
  • Classifies each family under existing ASAPPlanner owners: sketch selection, Hydra grouping, semantic rewriting, CSE, roll-up, accuracy reconciliation, Top-K reuse, and lifecycle planning.
  • Broadens the existing semantic-equivalent rewrite strategy to cover sum ∘ sum, sum ∘ count, min ∘ min, and max ∘ max.
  • Renames that existing owner to SemanticEquivalentRewriteStrategy; AvgToSumOverCountStrategy remains a compatibility export.
  • Adds no PatternStrategy and no standalone aggregate-composition strategy or module.
  • Explicitly excludes runtime cleanup policy and window-framework selection from this PR.

How

Temporal and spatial function recognition remains front-end lowering into typed AggIntent. Algorithm compatibility remains in SketchAlgorithmStrategy; subpopulation layout remains in HydraGroupingStrategy; equivalent logical algebra remains in the existing rewrite module; sharing and reuse remain in their current workload strategies; maintenance legality remains in lifecycle planning.

The rewrite requires an outer cross-entity reduction over an inner label-preserving per-entity reduction, one measure per node, no HAVING, and a registered algebraic law. It retains grouping, aliases, and accuracy, then accepts the result only if its output schema exactly matches the original.

For sum(count_over_time(...)), the rewrite takes the caller-visible name from the original output schema and inserts the required Float64 projection. This handles the PromQL front end's real empty output-name override instead of producing col_N and silently losing the candidate.

The planner may declare how long selected summary state must remain available and account for that requirement in legality and cost. Cleanup timers, eviction thresholds, garbage collection, and engine-specific recovery workarounds are owned by runtime/storage lifecycle management and are not imported as planner strategies.

Window-framework selection belongs in #331's physical deployment planning. That work should compare an extensible family of implementations—including tumbling windows, sliding windows, PromSketch-style exponential-histogram windows, and other window frameworks—rather than expose a closed tumbling/sliding enum from lifecycle code. This PR therefore adds no window candidate types or enumeration API.

Before

The collapsible temporal/spatial pairs were represented as nested operators but never proposed as equivalent rewrites. The initial version of this PR also missed real PromQL's empty output alias, had schema-only fixture assertions, and conflated lowering with executable summary coverage.

After

  • Actual PromQL-shaped sum(count_over_time(...)) is discovered by default workload search.
  • Tests verify the composed intent, grouping keys, preserved 5-minute TimeRange, raw Scan child, output schema, and default strategy registration.
  • Coverage distinguishes lowering, candidate generation, executable summary realization, and exact raw fallback. In particular, avg_over_time is documented as exact PassThrough, consistent with ASAPQuery's exact-only multi-stat fallback.
  • Cleanup policy remains runtime/storage responsibility.
  • Window-framework selection is deferred to feat(cost): model streaming summary resources #331 and is not constrained by types introduced here.

Validation

  • cargo test -p asap-aware-mapping on main — 240 passed
  • cargo test -p asap-integration-tests --test promql_to_post_asap — 3 passed
  • cargo test -p asap-devtools --bin dag_export post_asap_run_produces_both_summary_and_rewrite_replacements — passed
  • cargo fmt --all -- --check

Base

This PR is based directly on main; it has no dependency on #296 or the analytical-cost stack.

@zzylol
zzylol force-pushed the feat/asapquery-rule-coverage branch from 1acc47b to d1b6dc6 Compare September 2, 2026 21:45
@zzylol zzylol changed the title feat(planner): generalize aggregate composition rules feat(planner): classify ASAPQuery rules into existing strategies Sep 2, 2026
@zzylol
zzylol force-pushed the feat/asapquery-rule-coverage branch from d1b6dc6 to 2d3428c Compare September 2, 2026 21:46
@zzylol
zzylol marked this pull request as ready for review September 3, 2026 00:57
@zzylol
zzylol force-pushed the feat/asapquery-rule-coverage branch from 2d3428c to e21cce1 Compare September 3, 2026 01:30
@zzylol
zzylol changed the base branch from feat/dag-viewer-cost-annotations-286 to main September 3, 2026 01:31
@zzylol
zzylol force-pushed the feat/asapquery-rule-coverage branch 2 times, most recently from 588a4f7 to 3f5074c Compare September 3, 2026 01:45
@zzylol
zzylol force-pushed the feat/asapquery-rule-coverage branch from 3f5074c to 22f8757 Compare September 3, 2026 02:03
@zzylol

zzylol commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

TODO: sum ∘ count this rule may have counter example later.

@zzylol
zzylol merged commit afd9b59 into main Sep 3, 2026
4 checks passed
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.

2 participants