Metrics miscompile when a non-decomposable metric is in the same request

[shangyian]

Summary

When a request mixes a decomposable metric with a non-decomposable one (e.g. a percentile), DJ skips the per-group preaggregation for the whole grain group. The final SELECT still applies each component's merge function, but the components were never built by their accumulate function, so any component where accumulate != merge compiles to the wrong aggregate.

Symptoms

If we query various metric combinations together (over the same fact):

metric shape	accumulate → merge	result alongside a percentile
SUM(x), MIN, MAX	SUM → SUM	✅ (accumulate and merge coincide)
SUM(a) / SUM(b)	SUM → SUM	✅ (different columns, nothing collides)
AVG(x)	SUM → SUM and COUNT → SUM	❌ SUM(x) / SUM(x) returns 1.0
COUNT(*)	COUNT → SUM	❌ SUM(alias.*): invalid SQL
APPROX_COUNT_DISTINCT(x)	hll_sketch_agg → hll_union_agg	❌ MERGE applied to a raw column

Here's AVG with and without a percentile in the request:

-- AVG alone: correct, distinct pre-aggregated measures
CAST(SUM(t.x_sum_7ef98eef) AS DOUBLE) / NULLIF(NULLIF(SUM(t.x_count_7ef98eef), 0), 0)

-- AVG + percentile: numerator and denominator are the same column
CAST(SUM(t.x) AS DOUBLE) / NULLIF(NULLIF(SUM(t.x), 0), 0)

-- COUNT(*) alone:      SUM(t.count_d86193ee)
-- COUNT(*) + percentile: SUM(t.*)

-- APPROX_COUNT_DISTINCT alone:      CARDINALITY(MERGE(t.x_hll_4b72dd19))
-- APPROX_COUNT_DISTINCT + percentile: CARDINALITY(MERGE(t.x))

Root cause

In construction/build_v3/metrics.py, _build_base_metric_expression, states the invariant in its own docstring:

Always applies re-aggregation in the final SELECT using the component's merge function…
This is correct whether the CTE is at the exact requested grain or finer.

That holds only because the CTE is assumed to have already applied accumulate. When a non-decomposable metric sets skip_pre_agg, the CTE outputs raw rows and the assumption is false, but the outer select still emits merge-only.

Note that the trigger is not percentiles specifically -- it's anything that sets skip_pre_agg. The flag's comment mentions window functions as another case, which is worth checking for the same failure.

Proposed fix

When a grain group has not pre-aggregated, don't use the decomposition at all. Instead, emit the metric's original expression. DJ already has that branch and uses it for genuinely non-decomposable metrics:

Non-decomposable metrics (like MAX_BY): use original expression with CTE refs

Routing decomposable metrics down the same branch when their group skipped pre-aggregation fixes all four cases above: AVG(x) stays AVG(x), COUNT(*) stays COUNT(*), APPROX_COUNT_DISTINCT(x) stays itself.

We should also make the invariant explicit via a pre_aggregated flag on the grain group rather than inferring it from a component_aliases miss. The fix should add a regression test per accumulate != merge shape.

Reproducing

Using the existing example nodes, add a percentile metric over default.repair_orders_fact and request it alongside each of:

• default.avg_repair_price (avg(price)): expect SUM(price)/SUM(price)
• a count(...) metric on the same fact: expect SUM(alias.*)
• an approx_count_distinct(...) metric: expect MERGE over a raw column

Requesting each metric without the percentile produces correct SQL, which isolates the trigger.