Skip to content

Enforce Filter -> Aggregate.having pushdown as a canonicalization invariant #204

Open

Description

@zzylol

Summary

Per design principle 4 in docs/pre-asap-ir.md (predicates push down to the lowest node that can express them): a Filter whose predicate sits directly over an enclosing Aggregate's own output belongs in that Aggregate's having field, not as a separate Filter node wrapping it.

SELECT srcip, COUNT(*) AS cnt FROM packets GROUP BY srcip HAVING COUNT(*) > 10

should end up as:

Aggregate(reduction = Reduce(by = [srcip]), aggs = [Count], output_names = ["cnt"], having = Some(cnt > 10), child = Scan("packets"))

not:

Filter(pred = cnt > 10, child = Aggregate(reduction = Reduce(by = [srcip]), aggs = [Count], output_names = ["cnt"], having = None, child = Scan("packets")))

Current state

Nothing produces the first shape:

  • The SQL front end never populates having from a parsed HAVING clause (SQL front end never populates Aggregate.having from a real HAVING clause #201) — it (or an equivalent derived-table WHERE over an aggregate output) currently lowers straight to the Filter-wrapping shape.
  • crates/l2/src/canonicalize.rs has no rule that folds an existing Filter { pred, child: Aggregate { having: None, .. } } into Aggregate { having: Some(pred), .. } after the fact either.

Ask

Add a canonicalization pass rule (or equivalent invariant/sanity check) enforcing this: when a Filter's predicate resolves entirely against its immediate Aggregate child's own output columns, fold it into that Aggregate's having and drop the Filter node. This makes the invariant hold regardless of which front end or path produced the Filter { Aggregate } shape, and is the reason the pushdown decision matters — the L3->L4 binding pass reads having directly off Aggregate to know it can't be sketch-bound (needs the exact value, not an estimate); a wrapping Filter only reaches the same "stays logical" outcome today via a much blunter, unrelated rule (any logical node above a bindable aggregate falls back to logical).

Raised during review of #199 / design discussion referenced there (Milind Srivastava, Zeying Zhu) on where boolean conditions should live in the pre-ASAP IR: option (a) document the placement logic, option (b) enforce it in canonicalization. Zeying picked (b) for enforcement, with the docs update (a) landing in #199. Can land in a separate PR.

Activity

  1. added
    enhancementNew feature or request
    irPre-ASAP or post-ASAP intermediate representation
    canonicalizationCanonical IR normalization and invariants
    on Aug 26, 2026
  2. zzylol commented on Aug 26, 2026

    @zzylol
    ContributorAuthor

    Related open work

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

    canonicalizationCanonical IR normalization and invariantsenhancementNew feature or requestirPre-ASAP or post-ASAP intermediate representation

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions