hasura · hallettj · Mar 1, 2025 · Jan 22, 2025 · Jan 22, 2025 · Jan 24, 2025
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,9 +4,15 @@ This changelog documents the changes between release versions.
 
 ## [Unreleased v2]
 
+### Added
+
+- You can now group documents for aggregation according to multiple grouping criteria ([#144](https://github.com/hasura/ndc-mongodb/pull/144))
+
 ### Changed
 
 - **BREAKING:** Update to ndc-spec v0.2 ([#139](https://github.com/hasura/ndc-mongodb/pull/139))
+- **BREAKING:** Remove custom count aggregation - use standard count instead ([#144](https://github.com/hasura/ndc-mongodb/pull/144))
+- Results for `avg` and `sum` aggregations are coerced to consistent result types ([#144](https://github.com/hasura/ndc-mongodb/pull/144))
 
 #### ndc-spec v0.2
 
@@ -26,7 +32,23 @@ changelog](https://hasura.github.io/ndc-spec/specification/changelog.html#020).
 Use of the new spec requires a version of GraphQL Engine that supports ndc-spec
 v0.2, and there are required metadata changes.
 
+#### Removed custom count aggregation
+
+Previously there were two options for getting document counts named `count` and
+`_count`. These did the same thing. `count` has been removed - use `_count`
+instead.
+
+#### Results for `avg` and `sum` aggregations are coerced to consistent result types
+
+This change is required for compliance with ndc-spec.
+
+Results for `avg` are always coerced to `double`.
+
+Results for `sum` are coerced to `double` if the summed inputs use a fractional
+numeric type, or to `long` if inputs use an integral numeric type.
+
 ## [Unreleased v1]
+
 ### Added
 
 - Add uuid scalar type ([#148](https://github.com/hasura/ndc-mongodb/pull/148))

diff --git a/crates/configuration/src/mongo_scalar_type.rs b/crates/configuration/src/mongo_scalar_type.rs
@@ -1,7 +1,9 @@
+use std::fmt::Display;
+
 use mongodb_support::{BsonScalarType, EXTENDED_JSON_TYPE_NAME};
 use ndc_query_plan::QueryPlanError;
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, Hash, PartialEq, Eq)]
 pub enum MongoScalarType {
     /// One of the predefined BSON scalar types
     Bson(BsonScalarType),
@@ -40,3 +42,14 @@ impl TryFrom<&ndc_models::ScalarTypeName> for MongoScalarType {
         }
     }
 }
+
+impl Display for MongoScalarType {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            MongoScalarType::ExtendedJSON => write!(f, "extendedJSON"),
+            MongoScalarType::Bson(bson_scalar_type) => {
+                write!(f, "{}", bson_scalar_type.bson_name())
+            }
+        }
+    }
+}
diff --git a/crates/integration-tests/src/tests/aggregation.rs b/crates/integration-tests/src/tests/aggregation.rs
@@ -131,7 +131,7 @@ async fn returns_zero_when_counting_empty_result_set() -> anyhow::Result<()> {
               moviesAggregate(filter_input: {where: {title: {_eq: "no such movie"}}}) {
                 _count
                 title {
-                  count
+                  _count
                 }
               }
             }
@@ -152,7 +152,6 @@ async fn returns_zero_when_counting_nested_fields_in_empty_result_set() -> anyho
               moviesAggregate(filter_input: {where: {title: {_eq: "no such movie"}}}) {
                 awards {
                   nominations {
-                    count
                     _count
                   }
                 }

diff --git a/crates/integration-tests/src/tests/grouping.rs b/crates/integration-tests/src/tests/grouping.rs
@@ -0,0 +1,134 @@
+use insta::assert_yaml_snapshot;
+use ndc_test_helpers::{
+    asc, binop, column_aggregate, dimension_column, field, grouping, or, ordered_dimensions, query,
+    query_request, target, value,
+};
+
+use crate::{connector::Connector, run_connector_query};
+
+#[tokio::test]
+async fn runs_single_column_aggregate_on_groups() -> anyhow::Result<()> {
+    assert_yaml_snapshot!(
+        run_connector_query(
+            Connector::SampleMflix,
+            query_request().collection("movies").query(
+                query()
+                    // The predicate avoids an error when encountering documents where `year` is
+                    // a string instead of a number.
+                    .predicate(or([
+                        binop("_gt", target!("year"), value!(0)),
+                        binop("_lte", target!("year"), value!(0)),
+                    ]))
+                    .order_by([asc!("_id")])
+                    .limit(10)
+                    .groups(
+                        grouping()
+                            .dimensions([dimension_column("year")])
+                            .aggregates([
+                                (
+                                    "average_viewer_rating",
+                                    column_aggregate("tomatoes.viewer.rating", "avg"),
+                                ),
+                                ("max_runtime", column_aggregate("runtime", "max")),
+                            ])
+                            .order_by(ordered_dimensions()),
+                    ),
+            ),
+        )
+        .await?
+    );
+    Ok(())
+}
+
+#[tokio::test]
+async fn groups_by_multiple_dimensions() -> anyhow::Result<()> {
+    assert_yaml_snapshot!(
+        run_connector_query(
+            Connector::SampleMflix,
+            query_request().collection("movies").query(
+                query()
+                    .predicate(binop("_lt", target!("year"), value!(1950)))
+                    .order_by([asc!("_id")])
+                    .limit(10)
+                    .groups(
+                        grouping()
+                            .dimensions([
+                                dimension_column("year"),
+                                dimension_column("languages"),
+                                dimension_column("rated"),
+                            ])
+                            .aggregates([(
+                                "average_viewer_rating",
+                                column_aggregate("tomatoes.viewer.rating", "avg"),
+                            )])
+                            .order_by(ordered_dimensions()),
+                    ),
+            ),
+        )
+        .await?
+    );
+    Ok(())
+}
+
+#[tokio::test]
+async fn combines_aggregates_and_groups_in_one_query() -> anyhow::Result<()> {
+    assert_yaml_snapshot!(
+        run_connector_query(
+            Connector::SampleMflix,
+            query_request().collection("movies").query(
+                query()
+                    .predicate(binop("_gte", target!("year"), value!(2000)))
+                    .order_by([asc!("_id")])
+                    .limit(10)
+                    .aggregates([(
+                        "average_viewer_rating",
+                        column_aggregate("tomatoes.viewer.rating", "avg")
+                    )])
+                    .groups(
+                        grouping()
+                            .dimensions([dimension_column("year"),])
+                            .aggregates([(
+                                "average_viewer_rating_by_year",
+                                column_aggregate("tomatoes.viewer.rating", "avg"),
+                            )])
+                            .order_by(ordered_dimensions()),
+                    ),
+            ),
+        )
+        .await?
+    );
+    Ok(())
+}
+
+#[tokio::test]
+async fn combines_fields_and_groups_in_one_query() -> anyhow::Result<()> {
+    assert_yaml_snapshot!(
+        run_connector_query(
+            Connector::SampleMflix,
+            query_request().collection("movies").query(
+                query()
+                    // The predicate avoids an error when encountering documents where `year` is
+                    // a string instead of a number.
+                    .predicate(or([
+                        binop("_gt", target!("year"), value!(0)),
+                        binop("_lte", target!("year"), value!(0)),
+                    ]))
+                    .order_by([asc!("_id")])
+                    .limit(3)
+                    .fields([field!("title"), field!("year")])
+                    .order_by([asc!("_id")])
+                    .groups(
+                        grouping()
+                            .dimensions([dimension_column("year")])
+                            .aggregates([(
+                                "average_viewer_rating_by_year",
+                                column_aggregate("tomatoes.viewer.rating", "avg"),
+                            )])
+                            .order_by(ordered_dimensions()),
+                    )
+            ),
+        )
+        .await?
+    );
+    Ok(())
+}
diff --git a/crates/integration-tests/src/tests/local_relationship.rs b/crates/integration-tests/src/tests/local_relationship.rs
@@ -1,9 +1,10 @@
 use crate::{connector::Connector, graphql_query, run_connector_query};
 use insta::assert_yaml_snapshot;
 use ndc_test_helpers::{
-    asc, binop, exists, field, query, query_request, related, relation_field,
-    relationship, target, value,
+    asc, binop, column, column_aggregate, dimension_column, exists, field, grouping, is_in,
+    ordered_dimensions, query, query_request, related, relation_field, relationship, target, value,
 };
+use serde_json::json;
 
 #[tokio::test]
 async fn joins_local_relationships() -> anyhow::Result<()> {
@@ -243,3 +244,117 @@ async fn joins_relationships_on_nested_key() -> anyhow::Result<()> {
     );
     Ok(())
 }
+
+#[tokio::test]
+async fn groups_by_related_field() -> anyhow::Result<()> {
+    assert_yaml_snapshot!(
+        run_connector_query(
+            Connector::Chinook,
+            query_request()
+                .collection("Track")
+                .query(
+                    query()
+                        // avoid albums that are modified in mutation tests
+                        .predicate(is_in(
+                            target!("AlbumId"),
+                            [json!(15), json!(91), json!(227)]
+                        ))
+                        .groups(
+                            grouping()
+                                .dimensions([dimension_column(
+                                    column("Name").from_relationship("track_genre")
+                                )])
+                                .aggregates([(
+                                    "average_price",
+                                    column_aggregate("UnitPrice", "avg")
+                                )])
+                                .order_by(ordered_dimensions())
+                        )
+                )
+                .relationships([(
+                    "track_genre",
+                    relationship("Genre", [("GenreId", &["GenreId"])]).object_type()
+                )])
+        )
+        .await?
+    );
+    Ok(())
+}
+
+#[tokio::test]
+async fn gets_groups_through_relationship() -> anyhow::Result<()> {
+    assert_yaml_snapshot!(
+        run_connector_query(
+            Connector::Chinook,
+            query_request()
+                .collection("Album")
+                .query(
+                    query()
+                    // avoid albums that are modified in mutation tests
+                    .predicate(is_in(target!("AlbumId"), [json!(15), json!(91), json!(227)]))
+                    .order_by([asc!("_id")])
+                    .fields([field!("AlbumId"), relation_field!("tracks" => "album_tracks", query()
+                      .groups(grouping()
+                        .dimensions([dimension_column(column("Name").from_relationship("track_genre"))])
+                          .aggregates([
+                            ("AlbumId", column_aggregate("AlbumId", "avg")),
+                            ("average_price", column_aggregate("UnitPrice", "avg")),
+                          ])
+                          .order_by(ordered_dimensions()),
+                      )
+                    )])
+                )
+                .relationships([
+                    (
+                        "album_tracks",
+                        relationship("Track", [("AlbumId", &["AlbumId"])])
+                    ),
+                    (
+                        "track_genre",
+                        relationship("Genre", [("GenreId", &["GenreId"])]).object_type()
+                    )
+                ])
+        )
+        .await?
+    );
+    Ok(())
+}
+
+#[tokio::test]
+async fn gets_fields_and_groups_through_relationship() -> anyhow::Result<()> {
+    assert_yaml_snapshot!(
+        run_connector_query(
+            Connector::Chinook,
+            query_request()
+                .collection("Album")
+                .query(
+                    query()
+                    .predicate(is_in(target!("AlbumId"), [json!(15), json!(91), json!(227)]))
+                    .order_by([asc!("_id")])
+                    .fields([field!("AlbumId"), relation_field!("tracks" => "album_tracks", query()
+                      .order_by([asc!("_id")])
+                      .fields([field!("AlbumId"), field!("Name"), field!("UnitPrice")])
+                      .groups(grouping()
+                        .dimensions([dimension_column(column("Name").from_relationship("track_genre"))])
+                          .aggregates([(
+                            "average_price", column_aggregate("UnitPrice", "avg")
+                          )])
+                          .order_by(ordered_dimensions()),
+                      )
+                    )])
+                )
+                .relationships([
+                    (
+                        "album_tracks",
+                        relationship("Track", [("AlbumId", &["AlbumId"])])
+                    ),
+                    (
+                        "track_genre",
+                        relationship("Genre", [("GenreId", &["GenreId"])]).object_type()
+                    )
+                ])
+        )
+        .await?
+    );
+    Ok(())
+}
diff --git a/crates/integration-tests/src/tests/mod.rs b/crates/integration-tests/src/tests/mod.rs
@@ -11,6 +11,7 @@ mod aggregation;
 mod basic;
 mod expressions;
 mod filtering;
+mod grouping;
 mod local_relationship;
 mod native_mutation;
 mod native_query;
-Original file line number
+Diff line change
@@ Expand Up @@
                   moviesAggregate(filter_input: {where: {title: {_eq: "no such movie"}}}) {
                     _count
                     title {
-                      count
+                      _count
                     }
                   }
                 }
@@ Expand All @@
                   moviesAggregate(filter_input: {where: {title: {_eq: "no such movie"}}}) {
                     awards {
                       nominations {
-                        count
                         _count
                       }
                     }
@@ Expand Down @@