Skip to content

Commit eb3f404

Browse files
committed
Fix: window dim check takes priority over metric check in filter classification
1 parent 3af6d80 commit eb3f404

2 files changed

Lines changed: 96 additions & 9 deletions

File tree

sidemantic/sql/generator.py

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -735,21 +735,27 @@ def _classify_filters_for_pushdown(
735735
if dim and dim.window is not None:
736736
window_dim_models.add(clean_name)
737737

738-
# Filters that reference metrics must stay in main query (can't push down)
739-
# because metrics don't exist in CTEs (only _raw columns)
740-
if references_metric:
741-
main_query_filters.append(filter_expr)
742-
# Window-dim filters: kept separate so callers can handle them
743-
# appropriately. In the standard path they go to the outer WHERE;
744-
# in the preagg path they are pushed into each model's sub-query
745-
# (which has its own outer WHERE after window evaluation).
738+
# Window-dim filters take priority: kept separate so callers can
739+
# handle them appropriately. In the standard path they go to the
740+
# outer WHERE; in the preagg path they are pushed into each model's
741+
# sub-query (which has its own outer WHERE after window evaluation).
742+
# This check must come BEFORE the metric check because a filter
743+
# that references both a metric and a window dimension (e.g.,
744+
# "orders.next_status = 'complete' OR orders.revenue > 100") must
745+
# NOT land in main_query_filters/shared_filters: preagg CTEs do
746+
# not project window dimension columns, so applying the filter on
747+
# the outer preagg join would reference a non-existent column.
746748
# For multi-model filters, route to each model that owns a window
747749
# dim column so the preagg path includes them in the correct
748750
# sub-queries (the recursive generate() call will join in any
749751
# additional models referenced by the filter).
750-
elif window_dim_models:
752+
if window_dim_models:
751753
for wdm in window_dim_models:
752754
window_dim_filters[wdm].append(filter_expr)
755+
# Filters that reference metrics must stay in main query (can't push down)
756+
# because metrics don't exist in CTEs (only _raw columns)
757+
elif references_metric:
758+
main_query_filters.append(filter_expr)
753759
# If filter references exactly one model and no metrics, push it down
754760
elif len(referenced_models) == 1:
755761
model = list(referenced_models)[0]

tests/optimizations/test_predicate_pushdown.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -685,3 +685,84 @@ def test_multi_model_window_dim_filter_excluded_from_preagg_shared_filters(layer
685685
preagg_end = sql.index("order_items_preagg AS (")
686686
orders_subquery = sql[preagg_start:preagg_end]
687687
assert "next_status" in orders_subquery, "Multi-model window-dim filter should be pushed into orders sub-query"
688+
689+
690+
def test_mixed_metric_and_window_dim_filter_excluded_from_preagg_shared_filters(layer):
691+
"""Test that a filter referencing both a metric and a window dim goes to window_dim_filters.
692+
693+
When a filter like "orders.next_status = 'complete' OR orders.revenue > 100"
694+
references both a window dimension (next_status) and a metric (revenue), the
695+
window dim check must take priority over the metric check. Otherwise the filter
696+
lands in main_query_filters/shared_filters and gets rewritten onto preagg CTEs
697+
that don't project window dimension columns, causing a runtime failure.
698+
"""
699+
from sidemantic.core.model import Relationship
700+
701+
orders = Model(
702+
name="orders",
703+
table="orders_table",
704+
primary_key="order_id",
705+
dimensions=[
706+
Dimension(name="status", type="categorical", sql="status"),
707+
Dimension(
708+
name="next_status",
709+
type="categorical",
710+
sql="status",
711+
window="LEAD(status) OVER (PARTITION BY customer_id ORDER BY created_at)",
712+
),
713+
Dimension(name="order_date", type="time", sql="order_date", granularity="day"),
714+
],
715+
metrics=[
716+
Metric(name="revenue", agg="sum", sql="amount"),
717+
],
718+
relationships=[
719+
Relationship(
720+
name="order_items",
721+
type="one_to_many",
722+
sql="order_id",
723+
foreign_key="order_id",
724+
),
725+
],
726+
)
727+
728+
order_items = Model(
729+
name="order_items",
730+
table="order_items_table",
731+
primary_key="item_id",
732+
dimensions=[],
733+
metrics=[
734+
Metric(name="quantity", agg="sum", sql="qty"),
735+
],
736+
relationships=[
737+
Relationship(
738+
name="orders",
739+
type="many_to_one",
740+
foreign_key="order_id",
741+
primary_key="order_id",
742+
),
743+
],
744+
)
745+
746+
layer.add_model(orders)
747+
layer.add_model(order_items)
748+
749+
# Filter references BOTH a window dim (next_status) and a metric (revenue)
750+
sql = layer.compile(
751+
metrics=["orders.revenue", "order_items.quantity"],
752+
dimensions=["orders.order_date"],
753+
filters=["orders.next_status = 'complete' OR orders.revenue > 100"],
754+
)
755+
756+
# Should use pre-aggregation path
757+
assert "orders_preagg" in sql, "Should use pre-aggregation path"
758+
assert "order_items_preagg" in sql, "Should use pre-aggregation path"
759+
760+
# The mixed filter must NOT appear in the outer WHERE referencing _preagg
761+
# because preagg CTEs don't project window dimension columns
762+
assert "orders_preagg.next_status" not in sql, "Mixed metric/window-dim filter should NOT reference preagg CTE"
763+
764+
# The filter should be pushed into the orders sub-query instead
765+
preagg_start = sql.index("orders_preagg AS (")
766+
preagg_end = sql.index("order_items_preagg AS (")
767+
orders_subquery = sql[preagg_start:preagg_end]
768+
assert "next_status" in orders_subquery, "Mixed metric/window-dim filter should be pushed into orders sub-query"

0 commit comments

Comments
 (0)