execute_sql fan-trap guard false-refuses duplication-insensitive aggregates over a one-side column: COUNT(DISTINCT pk), MIN/MAX, and mixed-grain expressions (preflight_refused / risk: fan_trap)

[sandeep-agami]

Summary

The execute_sql fan-trap pre-flight guard (preflight_refused, risk: fan_trap) refuses (or silently auto-rewrites) a query the moment any column from the one-side table appears inside an aggregate function, regardless of which aggregate function it is or whether that column is actually the measure. A fan-out (one-to-many join) only inflates aggregates that are sensitive to row duplication — SUM, AVG, plain COUNT. It cannot change a duplication-insensitive aggregate: COUNT(DISTINCT <one-side pk>), MIN(...), MAX(...) are invariant under repeated rows. The guard does not make that distinction, so it wrongly refuses a set of extremely common analytical queries.

To be clear about scope: the guard already attributes an aggregate to its source table (_aggregate_source_tables) and correctly allows the common star-schema pattern of summing a many-side fact across a join (SELECT c.region, SUM(o.amount) ... GROUP BY c.region passes today, and there is a test for it). This issue is the residual precision gap on top of that: the classification keys off "a one-side column appears inside an aggregate" rather than "a duplication-sensitive rollup of a one-side measure."

The same class of over-refusal as #148 and #150: the guard acts before the SQL has been analyzed precisely enough to justify the action.

Environment

• Component: packages/agami-core/src/semantic_model/runtime.py (pre_flight_check → _preflight_select), surfaced by packages/agami-core/src/execute_sql.py.
• Reproduced directly against pre_flight_check on current main (see below — every classification is the actual output).

Reproduction (generic tables)

Model: customers (one) → orders (many) on orders.customer_id = customers.id.

False positives — these are refused today but are provably exact

-- FP1: distinct count of the one-side PK across the fan-out.
-- COUNT(DISTINCT) de-duplicates, so the fan-out cannot inflate it.
SELECT c.region, COUNT(DISTINCT c.id) AS n
FROM customers c JOIN orders o ON o.customer_id = c.id
WHERE o.status = 'won'
GROUP BY c.region;
-- actual: preflight_refused, risk=fan_trap  (WRONG — should run)

-- FP2: MIN/MAX of a one-side column. Idempotent under row duplication;
-- repeating a customer's row N times does not change MIN/MAX.
SELECT c.region, MIN(c.signup_date)
FROM customers c JOIN orders o ON o.customer_id = c.id
GROUP BY c.region;
-- actual: preflight_refused, risk=fan_trap  (WRONG — should run)

-- FP3: mixed-column aggregate. The measure is per-order (many side);
-- the one-side column is only a scalar co-factor. Grain is the many side,
-- so there is no duplication of a one-side measure.
SELECT SUM(o.amount * c.fx_rate)
FROM customers c JOIN orders o ON o.customer_id = c.id;
-- actual: preflight_refused, risk=fan_trap  (WRONG — should run)

Genuine fan trap — correctly flagged, and should stay flagged

-- A SUM of a ONE-side additive measure across the fan-out really does
-- multiply by the child count (each customer row repeats once per order).
SELECT SUM(c.employee_count)
FROM customers c JOIN orders o ON o.customer_id = c.id;
-- actual: risk=fan_trap, action=auto_rewrite  (CORRECT)

The inflation here is real (illustrative magnitudes): SELECT SUM(employee_count) FROM customers might be ~1.6M, while the same sum across the join is ~16x that, because each customer is repeated once per order. Fan traps are a genuine hazard — the intent of the guard is right; only its precision is wrong.

Already correct on main (shown to scope this issue)

-- Summing the MANY-side fact across the join is exact and is allowed today.
SELECT c.region, SUM(o.amount)
FROM customers c JOIN orders o ON o.customer_id = c.id
GROUP BY c.region;
-- actual: action=allow  (CORRECT — this is NOT the bug)

Observed vs expected

Query	Observed	Expected
COUNT(DISTINCT one_side.pk) across join	preflight_refused fan_trap	allow (distinct de-dupes)
MIN/MAX(one_side.col) across join	preflight_refused fan_trap	allow (idempotent under duplication)
SUM(many_side.measure * one_side.factor)	preflight_refused fan_trap	allow (measure grain is the many side)
SUM(one_side.additive_measure) across join	fan_trap (rewrite/refuse)	fan_trap — keep
SUM(many_side.fact) grouped by one-side dim	allow	allow — keep

Root cause

_preflight_select FAN branch — packages/agami-core/src/semantic_model/runtime.py:960-1006:

# FAN: an aggregate over a measure on the ONE side of a one-to-many in scope
for measure_table in agg_sources:
    others = table_set - {measure_table}
    fan_rels = _one_side_facing_many(rels, measure_table, others)
    if not fan_rels:
        continue
    ...
    # -> refuse (or auto_rewrite): "fan trap: aggregating '<one>' (one side)
    #    across a join to [<many>] (many side)."

agg_sources comes from _aggregate_source_tables — runtime.py:1397-1406:

def _aggregate_source_tables(tree, scope):
    """Tables whose columns appear inside an aggregate function in the SELECT."""
    sources = set()
    for select_expr in tree.expressions:
        for agg in select_expr.find_all(exp.AggFunc):
            for col in agg.find_all(exp.Column):
                t = _resolve_col_table(col, scope)
                if t:
                    sources.add(t)
    return sources

Two precision gaps follow from this:

1. No aggregate-function-class check. A table is treated as a fan-trap "measure on the one side" if any of its columns appears inside any exp.AggFunc. The specific function is never inspected, so COUNT(DISTINCT c.id), MIN(c.x), MAX(c.x) — all invariant under row duplication — are treated identically to SUM(c.x). The one-to-many cardinality is then confirmed by _one_side_facing_many (runtime.py:861-870) and the refusal is emitted at execute_sql.py:966-972.

2. No measure-vs-co-factor distinction. Because any column inside the aggregate contributes to agg_sources, a one-side column used only as a scalar factor of a many-side measure (SUM(o.amount * c.fx_rate)) pulls the one-side table into agg_sources, even though the aggregate's grain is the many side.

The decision is therefore made from (one-side column appears in some aggregate) × (one-to-many cardinality), not from (a duplication-sensitive rollup of a one-side measure) × (one-to-many cardinality).

Impact

Refuses several of the most common analytical shapes, e.g.:

• "How many distinct customers ordered, by region" (COUNT(DISTINCT one_side.pk) grouped by a joined dimension).
• "Earliest/most-recent one-side attribute among the joined population" (MIN/MAX of a one-side column).
• Currency/unit conversions and any measure scaled by a one-side rate (SUM(many_side.measure * one_side.factor)).

Tightening the classifier here also removes a chunk of user-visible preflight_refused noise without weakening the guard against real fan traps.

Suggested fix directions

Only treat a one-side table as a fan-trap measure source when the aggregate over it is duplication-sensitive:

• Aggregate-function class: exempt COUNT(DISTINCT <one-side col>), MIN(...), MAX(...) (and typically bool_and/bool_or, every) — these cannot change under a fan-out. Keep SUM, AVG, plain COUNT/COUNT(*), STDDEV, etc. as duplication-sensitive.
• Measure vs co-factor: only count a one-side table as a measure source when a one-side column is the aggregated value. When the aggregate expression also references a many-side column (mixed-column expression whose grain is the many side), do not classify it as a one-side fan trap.
• Pre-aggregated inputs: an aggregate over a subquery/CTE that is already grouped to the one-side grain is not a fan trap (this case already passes today; keep it a documented exemption so a future refactor does not regress it).

Suggested regression tests

Add alongside the existing test_aggregating_many_side_is_allowed in tests/test_semantic_model_runtime.py:

• COUNT(DISTINCT one_side.pk) across a one-to-many join → allow.
• MIN(one_side.col) / MAX(one_side.col), grouped by a joined dimension → allow.
• SUM(many_side.measure * one_side.factor) → allow.
• Guardrail (must stay red): SUM(one_side.additive_measure) across the join → fan_trap.

Related

• execute_sql default_filters rewrite corrupts valid SQL: scope-blind injection (CTEs, EXISTS subqueries) + dialect-blind INTERVAL mangling #148 — default_filters rewrite corrupts valid SQL (scope-blind / dialect-blind).
• execute_sql column-scope guard false-refuses valid date-math SQL: DATEDIFF/EXTRACT date-part keyword read as an out-of-scope column (column_out_of_scope) #150 — column-scope guard false-refuses valid date-math SQL.

Same underlying theme: analyze the SQL precisely before a guard refuses or rewrites it. The fan-trap hazard is real (the ~16x inflation above is genuine); the defect here is precision, not intent.