Skip to content

Commit

Permalink
implement aggregations on nested fields (#136)
Browse files Browse the repository at this point in the history
Turns on the nested field aggregation connector capability, and makes it work.

This also touches up the aggregation implementation generally. It uses the updated, unified system for referencing fields with paths. It also changes aggregation result types (besides count) to be nullable which avoids an error when aggregating over an empty document set.
  • Loading branch information
hallettj authored Dec 18, 2024
1 parent 8c457e6 commit a85094d
Show file tree
Hide file tree
Showing 22 changed files with 780 additions and 254 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,18 @@ This changelog documents the changes between release versions.

## [Unreleased]

### Added

- You can now aggregate values in nested object fields ([#136](https://github.com/hasura/ndc-mongodb/pull/136))

### Changed

- Result types for aggregation operations other than count are now nullable ([#136](https://github.com/hasura/ndc-mongodb/pull/136))

### Fixed

- Upgrade dependencies to get fix for RUSTSEC-2024-0421, a vulnerability in domain name comparisons ([#138](https://github.com/hasura/ndc-mongodb/pull/138))
- Aggregations on empty document sets now produce `null` instead of failing with an error ([#136](https://github.com/hasura/ndc-mongodb/pull/136))

#### Fix for RUSTSEC-2024-0421 / CVE-2024-12224

Expand Down
100 changes: 100 additions & 0 deletions crates/integration-tests/src/tests/aggregation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,103 @@ async fn aggregates_mixture_of_numeric_and_null_values() -> anyhow::Result<()> {
);
Ok(())
}

#[tokio::test]
async fn returns_null_when_aggregating_empty_result_set() -> anyhow::Result<()> {
assert_yaml_snapshot!(
graphql_query(
r#"
query {
moviesAggregate(filter_input: {where: {title: {_eq: "no such movie"}}}) {
runtime {
avg
}
}
}
"#
)
.run()
.await?
);
Ok(())
}

#[tokio::test]
async fn returns_zero_when_counting_empty_result_set() -> anyhow::Result<()> {
assert_yaml_snapshot!(
graphql_query(
r#"
query {
moviesAggregate(filter_input: {where: {title: {_eq: "no such movie"}}}) {
_count
title {
count
}
}
}
"#
)
.run()
.await?
);
Ok(())
}

#[tokio::test]
async fn returns_zero_when_counting_nested_fields_in_empty_result_set() -> anyhow::Result<()> {
assert_yaml_snapshot!(
graphql_query(
r#"
query {
moviesAggregate(filter_input: {where: {title: {_eq: "no such movie"}}}) {
awards {
nominations {
count
_count
}
}
}
}
"#
)
.run()
.await?
);
Ok(())
}

#[tokio::test]
async fn aggregates_nested_field_values() -> anyhow::Result<()> {
assert_yaml_snapshot!(
graphql_query(
r#"
query {
moviesAggregate(
filter_input: {where: {title: {_in: ["Within Our Gates", "The Ace of Hearts"]}}}
) {
tomatoes {
viewer {
rating {
avg
}
}
critic {
rating {
avg
}
}
}
imdb {
rating {
avg
}
}
}
}
"#
)
.run()
.await?
);
Ok(())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: crates/integration-tests/src/tests/aggregation.rs
expression: "graphql_query(r#\"\n query {\n moviesAggregate(\n filter_input: {where: {title: {_in: [\"Within Our Gates\", \"The Ace of Hearts\"]}}}\n ) {\n tomatoes {\n viewer {\n rating {\n avg\n }\n }\n critic {\n rating {\n avg\n }\n }\n }\n imdb {\n rating {\n avg\n }\n }\n }\n }\n \"#).run().await?"
---
data:
moviesAggregate:
tomatoes:
viewer:
rating:
avg: 3.45
critic:
rating:
avg: ~
imdb:
rating:
avg: 6.65
errors: ~
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
source: crates/integration-tests/src/tests/aggregation.rs
expression: "graphql_query(r#\"\n query {\n moviesAggregate(filter_input: {where: {title: {_eq: \"no such movie\"}}}) {\n runtime {\n avg\n }\n }\n }\n \"#).run().await?"
---
data:
moviesAggregate:
runtime:
avg: ~
errors: ~
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
source: crates/integration-tests/src/tests/aggregation.rs
expression: "graphql_query(r#\"\n query {\n moviesAggregate(filter_input: {where: {title: {_eq: \"no such movie\"}}}) {\n _count\n title {\n count\n }\n }\n }\n \"#).run().await?"
---
data:
moviesAggregate:
_count: 0
title:
count: 0
errors: ~
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/integration-tests/src/tests/aggregation.rs
expression: "graphql_query(r#\"\n query {\n moviesAggregate(filter_input: {where: {title: {_eq: \"no such movie\"}}}) {\n awards {\n nominations {\n count\n _count\n }\n }\n }\n }\n \"#).run().await?"
---
data:
moviesAggregate:
awards:
nominations:
count: 0
_count: 0
errors: ~
42 changes: 28 additions & 14 deletions crates/mongodb-agent-common/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ mod tests {
{
"$facet": {
"avg": [
{ "$match": { "gpa": { "$exists": true, "$ne": null } } },
{ "$match": { "gpa": { "$ne": null } } },
{ "$group": { "_id": null, "result": { "$avg": "$gpa" } } },
],
"count": [
{ "$match": { "gpa": { "$exists": true, "$ne": null } } },
{ "$match": { "gpa": { "$ne": null } } },
{ "$group": { "_id": "$gpa" } },
{ "$count": "result" },
],
Expand All @@ -123,10 +123,17 @@ mod tests {
{
"$replaceWith": {
"aggregates": {
"avg": { "$getField": {
"field": "result",
"input": { "$first": { "$getField": { "$literal": "avg" } } },
} },
"avg": {
"$ifNull": [
{
"$getField": {
"field": "result",
"input": { "$first": { "$getField": { "$literal": "avg" } } },
}
},
null
]
},
"count": {
"$ifNull": [
{
Expand Down Expand Up @@ -180,24 +187,31 @@ mod tests {
{ "$match": { "gpa": { "$lt": 4.0 } } },
{
"$facet": {
"avg": [
{ "$match": { "gpa": { "$exists": true, "$ne": null } } },
{ "$group": { "_id": null, "result": { "$avg": "$gpa" } } },
],
"__ROWS__": [{
"$replaceWith": {
"student_gpa": { "$ifNull": ["$gpa", null] },
},
}],
"avg": [
{ "$match": { "gpa": { "$ne": null } } },
{ "$group": { "_id": null, "result": { "$avg": "$gpa" } } },
],
},
},
{
"$replaceWith": {
"aggregates": {
"avg": { "$getField": {
"field": "result",
"input": { "$first": { "$getField": { "$literal": "avg" } } },
} },
"avg": {
"$ifNull": [
{
"$getField": {
"field": "result",
"input": { "$first": { "$getField": { "$literal": "avg" } } },
}
},
null
]
},
},
"rows": "$__ROWS__",
},
Expand Down
Loading

0 comments on commit a85094d

Please sign in to comment.