fix aggregation of time-dependent layers

Author: thomasp85

src/plot/layer/geom/area.rs

@@ -59,6 +59,10 @@ impl GeomTrait for Area {
         true
     }
 
+    fn aggregate_domain_aesthetics(&self) -> &'static [&'static str] {
+        &["pos1"]
+    }
+
     fn needs_stat_transform(&self, _aesthetics: &Mappings) -> bool {
         true
     }
@@ -83,6 +87,7 @@ impl GeomTrait for Area {
                 parameters,
                 dialect,
                 aesthetic_ctx,
+                self.aggregate_domain_aesthetics(),
             )?
         } else {
             StatResult::Identity

src/plot/layer/geom/bar.rs

@@ -108,6 +108,7 @@ impl GeomTrait for Bar {
                 parameters,
                 dialect,
                 aesthetic_ctx,
+                self.aggregate_domain_aesthetics(),
             );
         }
         stat_bar_count(query, schema, aesthetics, group_by)

src/plot/layer/geom/line.rs

@@ -45,6 +45,10 @@ impl GeomTrait for Line {
         true
     }
 
+    fn aggregate_domain_aesthetics(&self) -> &'static [&'static str] {
+        &["pos1"]
+    }
+
     fn needs_stat_transform(&self, _aesthetics: &Mappings) -> bool {
         true
     }
@@ -69,6 +73,7 @@ impl GeomTrait for Line {
                 parameters,
                 dialect,
                 aesthetic_ctx,
+                self.aggregate_domain_aesthetics(),
             )?
         } else {
             StatResult::Identity

src/plot/layer/geom/mod.rs

@@ -205,6 +205,17 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync {
         false
     }
 
+    /// Aesthetics that the Aggregate stat must keep as group keys rather than
+    /// aggregating, even if their bound column is continuous. This is for
+    /// geoms like line/area/ribbon where one axis is the *domain* — the
+    /// natural group identity of each row — and the user expects "summarise
+    /// the other axis per domain value" without writing an explicit target.
+    ///
+    /// Default empty; line/area/ribbon override to `&["pos1"]`.
+    fn aggregate_domain_aesthetics(&self) -> &'static [&'static str] {
+        &[]
+    }
+
     /// Apply statistical transformation to the layer query.
     ///
     /// The default implementation dispatches to the Aggregate stat when
@@ -231,6 +242,7 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync {
                 parameters,
                 dialect,
                 aesthetic_ctx,
+                self.aggregate_domain_aesthetics(),
             );
         }
         Ok(StatResult::Identity)

src/plot/layer/geom/ribbon.rs

@@ -44,6 +44,10 @@ impl GeomTrait for Ribbon {
         true
     }
 
+    fn aggregate_domain_aesthetics(&self) -> &'static [&'static str] {
+        &["pos1"]
+    }
+
     fn needs_stat_transform(&self, _aesthetics: &Mappings) -> bool {
         true
     }
@@ -68,6 +72,7 @@ impl GeomTrait for Ribbon {
                 parameters,
                 dialect,
                 aesthetic_ctx,
+                self.aggregate_domain_aesthetics(),
             )?
         } else {
             StatResult::Identity

src/plot/layer/geom/stat_aggregate.rs

@@ -602,6 +602,7 @@ pub fn apply(
     parameters: &HashMap<String, ParameterValue>,
     dialect: &dyn SqlDialect,
     aesthetic_ctx: &AestheticContext,
+    domain_aesthetics: &[&'static str],
 ) -> Result<StatResult> {
     let raw = match parameters.get("aggregate") {
         None | Some(ParameterValue::Null) => return Ok(StatResult::Identity),
@@ -656,6 +657,15 @@ pub fn apply(
             Some(c) => c.to_string(),
             None => continue, // literals & annotation columns pass through
         };
+        // Geom-declared domain aesthetics (e.g. `pos1` for line/area/ribbon)
+        // always become group keys — they identify each row, never get
+        // aggregated, never get dropped.
+        if domain_aesthetics.contains(&aes.as_str()) {
+            if !kept_cols.contains(&col) {
+                kept_cols.push(col);
+            }
+            continue;
+        }
         let info = schema.iter().find(|c| c.name == col);
         let is_discrete = info.map(|c| c.is_discrete).unwrap_or(false);
         if is_discrete {
@@ -922,11 +932,31 @@ mod tests {
         schema: &Schema,
         group_by: &[String],
         dialect: &dyn SqlDialect,
+    ) -> Result<StatResult> {
+        run_with_domain(params, aes, schema, group_by, dialect, &[])
+    }
+
+    fn run_with_domain(
+        params: ParameterValue,
+        aes: &Mappings,
+        schema: &Schema,
+        group_by: &[String],
+        dialect: &dyn SqlDialect,
+        domain: &[&'static str],
     ) -> Result<StatResult> {
         let mut p = HashMap::new();
         p.insert("aggregate".to_string(), params);
         let ctx = cartesian_ctx();
-        apply("SELECT * FROM t", schema, aes, group_by, &p, dialect, &ctx)
+        apply(
+            "SELECT * FROM t",
+            schema,
+            aes,
+            group_by,
+            &p,
+            dialect,
+            &ctx,
+            domain,
+        )
     }
 
     fn arr(items: &[&str]) -> ParameterValue {
@@ -1138,6 +1168,7 @@ mod tests {
             &p,
             &InlineQuantileDialect,
             &ctx,
+            &[],
         )
         .unwrap();
         assert_eq!(result, StatResult::Identity);
@@ -1451,6 +1482,57 @@ mod tests {
         }
     }
 
+    #[test]
+    fn domain_aesthetic_kept_as_group_key_even_when_continuous() {
+        // Regression test for the line/area/ribbon case: the user writes
+        //   DRAW line ... SETTING aggregate => ('y:min', 'y:max')
+        // and expects pos1 (the continuous time-axis column) to be a group
+        // key, not a dropped numeric mapping. The geom declares pos1 as a
+        // domain aesthetic; the stat keeps it as a group column.
+        let mut aes = Mappings::new();
+        aes.insert("pos1", col("__ggsql_aes_pos1__"));
+        aes.insert("pos2", col("__ggsql_aes_pos2__"));
+        let schema = schema_for(&[
+            ("__ggsql_aes_pos1__", false), // continuous, would be dropped without the domain hint
+            ("__ggsql_aes_pos2__", false),
+        ]);
+        let result = run_with_domain(
+            arr(&["y:min", "y:max"]),
+            &aes,
+            &schema,
+            &[],
+            &InlineQuantileDialect,
+            &["pos1"],
+        )
+        .unwrap();
+        match result {
+            StatResult::Transformed {
+                query,
+                stat_columns,
+                consumed_aesthetics,
+                ..
+            } => {
+                // pos1 is in the GROUP BY, not aggregated.
+                assert!(
+                    query.contains("GROUP BY \"__ggsql_aes_pos1__\""),
+                    "{}",
+                    query
+                );
+                assert!(!query.contains("MIN(\"__ggsql_aes_pos1__\")"));
+                assert!(!query.contains("MAX(\"__ggsql_aes_pos1__\")"));
+                // pos2 is exploded into MIN and MAX branches.
+                assert!(query.contains("MIN(\"__ggsql_aes_pos2__\")"));
+                assert!(query.contains("MAX(\"__ggsql_aes_pos2__\")"));
+                // pos1 is NOT consumed (kept), pos2 IS consumed.
+                assert!(!consumed_aesthetics.contains(&"pos1".to_string()));
+                assert!(consumed_aesthetics.contains(&"pos2".to_string()));
+                // synthetic aggregate column emitted in the explosion case.
+                assert!(stat_columns.contains(&"aggregate".to_string()));
+            }
+            _ => panic!("expected Transformed"),
+        }
+    }
+
     #[test]
     fn explosion_with_range_geom_two_defaults() {
         // For ribbon: pos1 + pos2min (lower) + pos2max (upper).