Skip to content

refactor(asap-types): model aggregation kinds and subtypes with typed enums #670

Closed

Description

@milindsrivastava1997

This was generated by AI during triage.

Problem

AggregationConfig represents an aggregation with an AggregationType plus an unconstrained aggregation_sub_type: String. The string is overloaded across unrelated aggregation kinds: plain CMS uses sum/count, MinMax uses min/max, heap CMS uses topk while its weighting is stored separately in parameters, Increase uses values such as rate, and aggregations without a subtype use an empty string.

This makes invalid combinations representable and forces the factory, planner, capability matcher, deserializers, and tests to interpret the same strings independently. Empty-string sentinels also make it unclear whether a subtype is intentionally absent or simply missing.

Goal

Replace the stringly-typed aggregation-kind/subtype representation with a typed, discriminated model. At minimum, plain CountMinSketch must have an explicit SUM-versus-COUNT mode, and other aggregation-specific modes must not be representable under the wrong aggregation kind.

Requirements

  • Define typed variants for aggregation-specific modes, including plain CMS SUM/COUNT and MinMax min/max.
  • Decide how heap CMS topk and count_events should be represented rather than splitting semantics between subtype and an untyped parameter map.
  • Preserve or explicitly migrate the existing serialized configuration format. Existing valid configs must remain readable during the migration.
  • Move validation to the configuration seam so factories, planners, matchers, and workers consume the same typed value.
  • Make absence of a subtype explicit for aggregation kinds that do not use one; do not use empty strings as implicit sentinels in new code.
  • Preserve the intentional legacy MultipleSubpopulation behavior unless a separate compatibility decision is documented.
  • Add behavioral tests for parsing, serialization compatibility, factory selection, and capability matching.

Non-goals

This is a representation and validation refactor; it should not change the intended SUM/COUNT behavior delivered by #505 or the heap follow-up tracked by #666.

Related: #505, #665, and #666.

Activity

  1. milindsrivastava1997 commented on Sep 2, 2026

    @milindsrivastava1997
    ContributorAuthor

    This was generated by AI during triage.

    Clarification: plain CountMinSketch uses aggregation_sub_type with sum/count. CountMinSketchWithHeap uses aggregation_sub_type: topk to identify its top-k structure; its SUM-versus-COUNT weighting is currently controlled separately by parameters["count_events"] (true or omitted for COUNT, false for SUM). The interim checks should preserve this distinction. A future typed model should represent both dimensions explicitly rather than forcing heap CMS to use sum/count as its subtype.

  2. milindsrivastava1997 commented on Sep 2, 2026

    @milindsrivastava1997
    ContributorAuthor

    This was generated by AI during triage.

    Triage follow-ups from the CMS review:

    • Finding 856: The legacy MultipleSubpopulation + CountMinSketch factory arm hardcodes SUM semantics (count_events: false) and does not validate a nested subtype. This path uses aggregation_sub_type as the inner accumulator kind (CountMinSketch/cms), unlike canonical CountMinSketch, where it means sum/count. Changing it is therefore a compatibility/design decision; defer it to this typed aggregation model refactor.
    • Finding 744: Plain CMS subtype interpretation is duplicated between query-engine factory construction and asap-types capability matching. The helpers have different directions, and asap-types cannot depend on query-engine. The durable fix is to introduce a shared typed aggregation/subtype model or shared parser in this refactor, then make both callers consume it.

    Both findings are intentionally out of scope for the current CMS behavior fix.

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