Fix: apply window dim filters as outer WHERE in preagg path

Window dimension filters were only pushed into the model sub-query that
owned the window dimension, leaving other models' metrics unfiltered.
Now the window dim column is projected through the owning model's preagg
CTE and the filter is applied on the outer preagg WHERE, constraining
all models' results.

Author: nicosuave

sidemantic/sql/generator.py

@@ -1468,6 +1468,28 @@ def _generate_with_preaggregation(
             all_filters, all_model_names
         )
 
+        # Collect window dimension columns that need to be projected through
+        # preagg CTEs so the outer WHERE can reference them.
+        window_dim_extra_dims: dict[str, list[str]] = {}
+        for model_name, wf_list in window_dim_filters.items():
+            for f_expr in wf_list:
+                try:
+                    parsed_f = sqlglot.parse_one(f_expr, dialect=self.dialect)
+                    for col in parsed_f.find_all(exp.Column):
+                        clean = (col.table or "").replace("_cte", "")
+                        if clean in all_model_names:
+                            model_obj = self.graph.get_model(clean)
+                            if model_obj:
+                                dim_obj = model_obj.get_dimension(col.name)
+                                if dim_obj and dim_obj.window is not None:
+                                    dim_ref = f"{clean}.{col.name}"
+                                    if clean not in window_dim_extra_dims:
+                                        window_dim_extra_dims[clean] = []
+                                    if dim_ref not in window_dim_extra_dims[clean]:
+                                        window_dim_extra_dims[clean].append(dim_ref)
+                except Exception:
+                    pass
+
         # Generate a pre-aggregated CTE for each metric model
         preagg_ctes = []
         cte_names = []
@@ -1477,17 +1499,24 @@ def _generate_with_preaggregation(
             cte_names.append(cte_name)
 
             # Only pass filters relevant to this model's sub-query.
-            # Window-dim filters are included here (not in shared_filters) because
-            # preagg CTEs only project query dimensions/metrics, not window dims.
-            # The recursive generate() call handles them correctly in its outer WHERE.
-            model_filters = pushdown_by_model.get(model_name, []) + window_dim_filters.get(model_name, [])
+            # Window-dim filters are NOT pushed here: they are applied as outer
+            # WHERE on the final preagg join so that ALL models' metrics are
+            # constrained, not just the model that owns the window dimension.
+            model_filters = pushdown_by_model.get(model_name, [])
+
+            # Add extra window dim columns to the sub-query dimensions so they
+            # get projected through the preagg CTE for outer WHERE access.
+            sub_dimensions = list(dimensions)
+            for extra_dim in window_dim_extra_dims.get(model_name, []):
+                if extra_dim not in sub_dimensions:
+                    sub_dimensions.append(extra_dim)
 
             # Generate sub-query for this model's metrics at the dimension grain
             # We call generate() recursively but it won't trigger pre-aggregation
             # again because each sub-query has metrics from only one model
             sub_query = self.generate(
                 metrics=model_metrics,
-                dimensions=dimensions,
+                dimensions=sub_dimensions,
                 filters=model_filters,
                 segments=None,  # Already resolved
                 order_by=None,
@@ -1568,18 +1597,25 @@ def _generate_with_preaggregation(
 
         final_query = f"SELECT\n  {select_str}\nFROM {from_str}"
 
-        # Apply shared filters (cross-model or metric-level) on the outer query.
+        # Apply shared filters and window-dim filters on the outer query.
+        # Window-dim filters are applied here (not inside model sub-queries) so
+        # that ALL models' metrics are constrained by the filter, not just the
+        # model that owns the window dimension.
         # Rewrite table references from model/model_cte to model_preagg using
         # sqlglot AST rewriting so we don't accidentally mangle column names
         # that happen to contain a model name as a substring.
-        if shared_filters:
+        all_outer_filters = list(shared_filters)
+        for wf_list in window_dim_filters.values():
+            all_outer_filters.extend(wf_list)
+
+        if all_outer_filters:
             preagg_table_map = {}
             for model_name in metrics_by_model:
                 preagg_table_map[model_name] = f"{model_name}_preagg"
                 preagg_table_map[f"{model_name}_cte"] = f"{model_name}_preagg"
 
             rewritten = []
-            for f in shared_filters:
+            for f in all_outer_filters:
                 try:
                     parsed = sqlglot.parse_one(f, dialect=self.dialect)
                     for col in parsed.find_all(exp.Column):

tests/optimizations/test_predicate_pushdown.py

@@ -519,15 +519,13 @@ def test_window_dimension_filter_with_regular_filter(layer):
     pytest.main([__file__, "-v"])
 
 
-def test_window_dim_filter_excluded_from_preagg_shared_filters(layer):
-    """Test that window-dim filters do not become shared_filters in the preagg path.
+def test_window_dim_filter_applied_as_outer_where_in_preagg(layer):
+    """Test that window-dim filters are applied as outer WHERE in the preagg path.
 
     When metrics come from multiple models (triggering pre-aggregation), a filter
-    on a window dimension must NOT be applied as a shared filter on the outer
-    query over preagg CTEs, because those CTEs only project query dimensions
-    and metrics, not window dimension columns. Instead, the window-dim filter
-    should be pushed into the relevant model's sub-query where the recursive
-    generate() call can handle it via its own outer WHERE.
+    on a window dimension must be applied as an outer WHERE on the final preagg
+    join so that ALL models' metrics are constrained. The window dim column is
+    projected through the owning model's preagg CTE to make it available.
     """
     from sidemantic.core.model import Relationship
 
@@ -590,28 +588,26 @@ def test_window_dim_filter_excluded_from_preagg_shared_filters(layer):
     assert "orders_preagg" in sql, "Should use pre-aggregation path"
     assert "order_items_preagg" in sql, "Should use pre-aggregation path"
 
-    # The window-dim filter must NOT appear in the outer WHERE referencing _preagg
-    # because preagg CTEs don't project the window dimension column
-    assert "orders_preagg.next_status" not in sql, (
-        "Window-dim filter should NOT reference preagg CTE (column doesn't exist there)"
-    )
-
-    # The filter should be pushed into the orders sub-query instead
-    # (visible inside the orders_preagg CTE definition)
+    # The window dim column should be projected in the orders preagg CTE
     preagg_start = sql.index("orders_preagg AS (")
     preagg_end = sql.index("order_items_preagg AS (")
     orders_subquery = sql[preagg_start:preagg_end]
-    assert "next_status" in orders_subquery, "Window-dim filter should be pushed into orders sub-query"
+    assert "next_status" in orders_subquery, "Window dim column should be projected in orders preagg CTE"
+
+    # The filter should appear in the outer WHERE referencing the preagg CTE,
+    # so that BOTH models' metrics are constrained by the filter
+    outer_query = sql[sql.rindex("SELECT") :]
+    assert "orders_preagg.next_status" in outer_query, (
+        "Window-dim filter should be in outer WHERE referencing preagg CTE"
+    )
 
 
-def test_multi_model_window_dim_filter_excluded_from_preagg_shared_filters(layer):
-    """Test that multi-model window-dim filters do not become shared_filters in the preagg path.
+def test_multi_model_window_dim_filter_applied_as_outer_where_in_preagg(layer):
+    """Test that multi-model window-dim filters are applied as outer WHERE in preagg.
 
     When a filter references columns from multiple models and at least one is a
-    window dimension, it must NOT be applied as a shared filter on preagg CTEs
-    (which only project query dimensions/metrics, not window columns). Instead it
-    should be routed into window_dim_filters for the model that owns the window
-    dimension, so the recursive generate() call can handle it.
+    window dimension, the window dim column is projected through the preagg CTE
+    and the filter is applied on the outer WHERE so both models are constrained.
     """
     from sidemantic.core.model import Relationship
 
@@ -676,25 +672,24 @@ def test_multi_model_window_dim_filter_excluded_from_preagg_shared_filters(layer
     assert "orders_preagg" in sql, "Should use pre-aggregation path"
     assert "order_items_preagg" in sql, "Should use pre-aggregation path"
 
-    # The multi-model window-dim filter must NOT appear in the outer WHERE
-    # referencing _preagg CTEs, because preagg CTEs don't project window columns
-    assert "orders_preagg.next_status" not in sql, "Multi-model window-dim filter should NOT reference preagg CTE"
-
-    # The filter should be pushed into the orders sub-query instead
+    # The window dim column should be projected in the orders preagg CTE
     preagg_start = sql.index("orders_preagg AS (")
     preagg_end = sql.index("order_items_preagg AS (")
     orders_subquery = sql[preagg_start:preagg_end]
-    assert "next_status" in orders_subquery, "Multi-model window-dim filter should be pushed into orders sub-query"
+    assert "next_status" in orders_subquery, "Window dim column should be projected in orders preagg CTE"
+
+    # The filter should appear in the outer WHERE
+    outer_query = sql[sql.rindex("SELECT") :]
+    assert "orders_preagg.next_status" in outer_query, "Multi-model window-dim filter should be in outer WHERE"
 
 
-def test_mixed_metric_and_window_dim_filter_excluded_from_preagg_shared_filters(layer):
-    """Test that a filter referencing both a metric and a window dim goes to window_dim_filters.
+def test_mixed_metric_and_window_dim_filter_applied_as_outer_where_in_preagg(layer):
+    """Test that a filter referencing both a metric and a window dim is applied as outer WHERE.
 
     When a filter like "orders.next_status = 'complete' OR orders.revenue > 100"
     references both a window dimension (next_status) and a metric (revenue), the
-    window dim check must take priority over the metric check. Otherwise the filter
-    lands in main_query_filters/shared_filters and gets rewritten onto preagg CTEs
-    that don't project window dimension columns, causing a runtime failure.
+    window dim check takes priority and the filter is applied on the outer preagg
+    WHERE. The window dim column is projected through the preagg CTE.
     """
     from sidemantic.core.model import Relationship
 
@@ -757,12 +752,12 @@ def test_mixed_metric_and_window_dim_filter_excluded_from_preagg_shared_filters(
     assert "orders_preagg" in sql, "Should use pre-aggregation path"
     assert "order_items_preagg" in sql, "Should use pre-aggregation path"
 
-    # The mixed filter must NOT appear in the outer WHERE referencing _preagg
-    # because preagg CTEs don't project window dimension columns
-    assert "orders_preagg.next_status" not in sql, "Mixed metric/window-dim filter should NOT reference preagg CTE"
-
-    # The filter should be pushed into the orders sub-query instead
+    # The window dim column should be projected in the orders preagg CTE
     preagg_start = sql.index("orders_preagg AS (")
     preagg_end = sql.index("order_items_preagg AS (")
     orders_subquery = sql[preagg_start:preagg_end]
-    assert "next_status" in orders_subquery, "Mixed metric/window-dim filter should be pushed into orders sub-query"
+    assert "next_status" in orders_subquery, "Window dim column should be projected in orders preagg CTE"
+
+    # The filter should be in the outer WHERE, constraining both models
+    outer_query = sql[sql.rindex("SELECT") :]
+    assert "orders_preagg.next_status" in outer_query, "Mixed metric/window-dim filter should be in outer WHERE"