Fix: exclude window dimensions from pre-aggregation materialization

Window dimensions (LEAD, LAG, etc.) are incompatible with GROUP BY in
rollup materialization SQL. generate_materialization_sql now raises a
clear ValueError when a pre-aggregation references a dimension that has
a window expression set, for both regular dimensions and time dimensions.

Author: nicosuave

sidemantic/core/pre_aggregation.py

@@ -143,16 +143,28 @@ def generate_materialization_sql(self, model: Any) -> str:
         if self.time_dimension and self.granularity:
             time_dim = model.get_dimension(self.time_dimension)
             if time_dim:
+                if time_dim.window:
+                    raise ValueError(
+                        f"Cannot use window dimension '{self.time_dimension}' as time_dimension "
+                        f"in pre-aggregation '{self.name}': window functions are incompatible "
+                        f"with GROUP BY in rollup materialization"
+                    )
                 col_name = f"{self.time_dimension}_{self.granularity}"
                 select_exprs.append(f"DATE_TRUNC('{self.granularity}', {time_dim.sql_expr}) as {col_name}")
                 group_by_positions.append(str(pos))
                 pos += 1
 
-        # Add dimensions
+        # Add dimensions (reject window dimensions - incompatible with GROUP BY)
         if self.dimensions:
             for dim_name in self.dimensions:
                 dim = model.get_dimension(dim_name)
                 if dim:
+                    if dim.window:
+                        raise ValueError(
+                            f"Cannot use window dimension '{dim_name}' in pre-aggregation "
+                            f"'{self.name}': window functions are incompatible with "
+                            f"GROUP BY in rollup materialization"
+                        )
                     select_exprs.append(f"{dim.sql_expr} as {dim_name}")
                     group_by_positions.append(str(pos))
                     pos += 1

tests/optimizations/test_pre_aggregations.py

@@ -1204,5 +1204,100 @@ def test_preagg_injection_in_preagg_name_rejected():
         preagg.get_table_name("orders")
 
 
+def test_generate_materialization_sql_rejects_window_dimension():
+    """Test that generate_materialization_sql raises ValueError for window dimensions."""
+    model = Model(
+        name="events",
+        table="events",
+        dimensions=[
+            Dimension(name="event", type="categorical", sql="event"),
+            Dimension(
+                name="next_event",
+                type="categorical",
+                sql="event",
+                window="LEAD(event) OVER (PARTITION BY person_id ORDER BY timestamp)",
+            ),
+            Dimension(name="status", type="categorical", sql="status"),
+        ],
+        metrics=[
+            Metric(name="count", agg="count"),
+        ],
+    )
+
+    # Pre-aggregation that references a window dimension should raise
+    preagg = PreAggregation(
+        name="by_next_event",
+        measures=["count"],
+        dimensions=["next_event"],
+    )
+
+    with pytest.raises(ValueError, match="window dimension.*next_event.*incompatible"):
+        preagg.generate_materialization_sql(model)
+
+
+def test_generate_materialization_sql_rejects_window_time_dimension():
+    """Test that generate_materialization_sql raises ValueError for window time dimensions."""
+    model = Model(
+        name="events",
+        table="events",
+        dimensions=[
+            Dimension(
+                name="next_timestamp",
+                type="time",
+                sql="timestamp",
+                window="LEAD(timestamp) OVER (PARTITION BY person_id ORDER BY timestamp)",
+            ),
+        ],
+        metrics=[
+            Metric(name="count", agg="count"),
+        ],
+    )
+
+    preagg = PreAggregation(
+        name="daily_next",
+        measures=["count"],
+        dimensions=[],
+        time_dimension="next_timestamp",
+        granularity="day",
+    )
+
+    with pytest.raises(ValueError, match="window dimension.*next_timestamp.*incompatible"):
+        preagg.generate_materialization_sql(model)
+
+
+def test_generate_materialization_sql_normal_dimensions_still_work():
+    """Test that pre-aggregations with normal (non-window) dimensions still work."""
+    model = Model(
+        name="events",
+        table="events",
+        dimensions=[
+            Dimension(name="event", type="categorical", sql="event"),
+            Dimension(name="status", type="categorical", sql="status"),
+            Dimension(
+                name="next_event",
+                type="categorical",
+                sql="event",
+                window="LEAD(event) OVER (PARTITION BY person_id ORDER BY timestamp)",
+            ),
+        ],
+        metrics=[
+            Metric(name="count", agg="count"),
+        ],
+    )
+
+    # Pre-aggregation with only normal dimensions should work fine
+    preagg = PreAggregation(
+        name="by_status",
+        measures=["count"],
+        dimensions=["status"],
+    )
+
+    sql = preagg.generate_materialization_sql(model)
+    assert "status as status" in sql
+    assert "GROUP BY" in sql
+    assert "LEAD" not in sql
+    assert "OVER" not in sql
+
+
 if __name__ == "__main__":
     pytest.main([__file__, "-v"])