Skip to content

Concat drops unique_keys unconditionally — no way for a producer to assert cross-branch disjointness #228

Description

@zzylol

Summary

QueryExpr::Concat (the n-ary exact UNION ALL node, renamed from Merge in #226) always drops unique_keys on its output, even when every branch is individually deduplicated on the same key column(s). This is currently the only sound default — a key that's unique within one branch is not unique across the concatenation unless the branches' value sets for that key are provably disjoint, and nothing about matching schemas / matching per-branch unique keys establishes that. See merge_drops_the_branches_unique_keys and merge_and_setop_agree_on_unique_keys in crates/types/src/pre_asap/query_expr.rs, which pin this down: a Dedup branch's own unique key must not leak out through a Concat over it.

Concretely: PromQL histogram_quantiles (one branch per φ) and SQL ROLLUP/CUBE/GROUPING SETS (one branch per grouping level) both build Concat nodes where every branch happens to share the same key columns, but two different branches can still emit the same key value — e.g. two φ-branches both keyed on (host, le) can both produce a (host, le) pair.

Where this could theoretically be recovered

If a branch is tagged with an explicit discriminator column whose value is guaranteed to differ per branch (a literal, not inferred structurally — e.g. le/φ riding along as an output column with a distinct value per branch, or Postgres-style synthetic GROUPING() id for ROLLUP), then (discriminator, original_key) is a sound compound unique key of the concatenation, because the discriminator alone partitions rows into disjoint sets independent of the branches' actual data.

Concat::output_schema() itself has no way to discover this — it would require the constructor (the lowering code that builds the Concat, and is the only one who knows why these particular branches are being combined and whether it tagged them with a truly-distinct discriminator) to assert it explicitly.

Proposed direction (needs a decision, not just an implementation)

Two options, not obviously reconciled yet:

  1. Producer-supplied override: let the code building a Concat optionally assert its own unique_keys on the resulting node when it can prove branch disjointness (e.g. via a discriminator column it just added). Concat::output_schema()'s default (drop everything) stays for every other caller.
  2. Don't try to preserve it in the IR at all: if a downstream consumer needs the uniqueness guarantee, it re-establishes one explicitly with a Dedup node after the Concat. More verbose, but zero risk of a false claim leaking through — no new trust boundary to get wrong.

Before implementing either

Check whether any current histogram_quantiles / ROLLUP/CUBE/GROUPING SETS call site is actually paying for a redundant Dedup/equivalent it could prove unnecessary today. If nothing currently needs this, this may not be worth the added surface area — flagging as a design question to resolve, not a confirmed feature.

Related

Activity

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

    enhancementNew feature or requestirPre-ASAP or post-ASAP intermediate representationschemaSchema, type, key, and nullability contracts

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions