Skip to content

SQL per-entity fold over an estimate has no guarantee: SELECT src, max(p) … GROUP BY src over a DDSketch p #751

Open

Description

@zzylol

Problem, by example

SELECT src, max(p)
FROM (SELECT src, approx_percentile_cont(bytes, 0.99) AS p FROM flows GROUP BY src) s
GROUP BY src

over flows(src Utf8, bytes Float64), with target AccuracyTarget::max_error(0.01). The inner query has one row per src, so each outer group holds exactly one p, and max(p) returns that value. The DDSketch candidate's p is within relative 0.01 (α = 0.01), so the outer result is within relative 0.01 too.

What Stage 1 composes today (run through asap_integration_tests::planning::logical_candidates at 20d2923, filtered to the candidates that read a DDSketch):

Query Root guarantee today Sound guarantee
inner alone: SELECT src, approx_percentile_cont(bytes, 0.99) AS p FROM flows GROUP BY src relative, error 0.01 relative 0.01
global fold: SELECT max(p) FROM (…) s relative, error max(0.01), δ over group_row_count same
per-src fold: SELECT src, max(p) FROM (…) s GROUP BY src None relative 0.01

So a per-entity fold over an estimate has no guarantee, and plan selection rejects every approximate candidate for it; only exact plans remain.

Why it happens

  1. The SQL frontend lowers the outer GROUP BY src to Aggregate { reduction: Reduce(GroupKeys { keys: [0, 1] }), measures: [Max { col: Some(1) }] } over (src, p): the group keys include p (column 1), presumably added because p is functionally dependent on the subquery's unique key src.
  2. groups_by_estimate (crates/logical-asap-optimization/src/per_computation_candidates.rs) sees a non-label, non-time group key (p, Float64) over approximate input and says the grouping may decide groups by an estimate, so composed_guarantee returns None.
  3. Even with the grouping recognized as one row per entity (Reduction::PerEntity), fold_stats gives PropagationStats for PerEntity only over a PromQL subquery with a resolution (rows = range / resolution). A SQL per-entity fold has no resolution, so it also returns None.

Proposal

  • Recognize a fold whose group keys contain a unique key of its input (src, the inner aggregate's group key) as one row per group: keys functionally dependent on that key (here p) do not decide the grouping, so groups_by_estimate is false for them.
  • Give such a fold a row count: fold_stats returns input_row_count: Some(1) for a per-entity fold over a SQL subquery (each group holds one input row), as it returns range / resolution for a PromQL subquery. The relative fold then composes max(0.01) with δ unchanged.
  • Alternative: have the SQL frontend lower a GROUP BY over the input's unique key to Reduction::PerEntity, and resolve the row count in fold_stats only for that.

Scope and acceptance

Status and related work

Raised as a review note on the SQL fold composition change (#687). Related: #689 (column-level guarantees).

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    accuracyAccuracy requirements, guarantees, and propagationenhancementNew feature or requestnested queriessqlSQL front-end lowering (DataFusion → L2)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions