Skip to content

fix: fan-out-safe t.all() totals and loud, ownership-based filter pus…#283

Merged
hachej merged 1 commit into
mainfrom
fix/preagg-filter-and-totals
Jul 8, 2026
Merged

fix: fan-out-safe t.all() totals and loud, ownership-based filter pus…#283
hachej merged 1 commit into
mainfrom
fix/preagg-filter-and-totals

Conversation

@hussainsultan

Copy link
Copy Markdown
Collaborator

…hdown

Fixes three silent-wrong-answer defects in the join_many pre-aggregation path, found during a soundness evaluation of v0.3.15:

  1. t.all(...) totals were built by re-running agg specs on the fanned-out joined table, inflating denominators (percent_of_total summed to 0.5 with 2 line items per order). The pre-agg path now captures each (filtered) source table plus its original measure expressions and passes a lazy fan-out-safe totals builder through _apply_calc_specs → apply_calc_measures(totals_base_builder=...): each table aggregates at zero grain — where fan-out cannot occur — and the one-row results are cross-joined. Correct for sum/count, mean, and count-distinct totals.

  2. Filters were silently dropped or mis-pushed. Root cause: _Resolver .getitem ignored the dimension map, so t["orders.status"] raised and the entire filtered-join build collapsed to tbl=None inside a broad except, discarding the filter. Bracket access on _Resolver / _AggResolver now falls back to declared dims/measures (materialized columns still win, preserving post-agg stale-dimension semantics), and pre-agg filter pushdown is ownership-based: each filter resolves against every source table's dims (bare and table-prefixed); it is pushed to a raw table only when exactly one table owns it, ambiguous filters apply through the filtered joined table's key bridge, and a filter handled by neither path raises ValueError instead of being silently ignored. The deferred-join path's filter application and _find_deferrable_joins' reference detection get the same treatment.

  3. find_join_in_tree treated the wrapper SemanticTableOp created by SemanticJoin.with_measures()/with_dimensions() as a leaf, so a .filter() between the wrapper and .aggregate() silently skipped pre-aggregation and returned fanned-out sums. The wrapper's _source_join is now followed.

Regression coverage in test_preagg_filters_and_totals.py: totals under uneven 1:N fan (sum and mean), totals with filters, prefixed / bare- ambiguous / single-owner filter pushdown, unresolvable-filter error, and prefixed filters on the join_one path.

…hdown

Fixes three silent-wrong-answer defects in the join_many pre-aggregation
path, found during a soundness evaluation of v0.3.15:

1. t.all(...) totals were built by re-running agg specs on the fanned-out
   joined table, inflating denominators (percent_of_total summed to 0.5
   with 2 line items per order). The pre-agg path now captures each
   (filtered) source table plus its original measure expressions and
   passes a lazy fan-out-safe totals builder through _apply_calc_specs →
   apply_calc_measures(totals_base_builder=...): each table aggregates at
   zero grain — where fan-out cannot occur — and the one-row results are
   cross-joined. Correct for sum/count, mean, and count-distinct totals.

2. Filters were silently dropped or mis-pushed. Root cause: _Resolver
   .__getitem__ ignored the dimension map, so t["orders.status"] raised
   and the entire filtered-join build collapsed to tbl=None inside a
   broad except, discarding the filter. Bracket access on _Resolver /
   _AggResolver now falls back to declared dims/measures (materialized
   columns still win, preserving post-agg stale-dimension semantics),
   and pre-agg filter pushdown is ownership-based: each filter resolves
   against every source table's dims (bare and table-prefixed); it is
   pushed to a raw table only when exactly one table owns it, ambiguous
   filters apply through the filtered joined table's key bridge, and a
   filter handled by neither path raises ValueError instead of being
   silently ignored. The deferred-join path's filter application and
   _find_deferrable_joins' reference detection get the same treatment.

3. find_join_in_tree treated the wrapper SemanticTableOp created by
   SemanticJoin.with_measures()/with_dimensions() as a leaf, so a
   .filter() between the wrapper and .aggregate() silently skipped
   pre-aggregation and returned fanned-out sums. The wrapper's
   _source_join is now followed.

Regression coverage in test_preagg_filters_and_totals.py: totals under
uneven 1:N fan (sum and mean), totals with filters, prefixed / bare-
ambiguous / single-owner filter pushdown, unresolvable-filter error, and
prefixed filters on the join_one path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hachej hachej merged commit 6e68c62 into main Jul 8, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants