Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement aggregations on nested fields #136

Merged
merged 39 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
8d52274
start generating new supergraph
hallettj Dec 4, 2024
51aed31
introspect sample_mflix
hallettj Dec 4, 2024
bc1e9f1
connector-link
hallettj Dec 4, 2024
8a4430d
I'm told I need to update compatibility-config
hallettj Dec 4, 2024
f9c4755
increase introspection sample size
hallettj Dec 4, 2024
8f9143a
init the other connectors
hallettj Dec 4, 2024
b73405c
introspect and link chinook
hallettj Dec 4, 2024
a9df60d
introspect and link test_cases
hallettj Dec 4, 2024
08f015a
copy over relationship and command configurations
hallettj Dec 5, 2024
88c0555
update nix configuration
hallettj Dec 5, 2024
a9d1e6f
remove secret tokens and authorization headers
hallettj Dec 5, 2024
e25bd1e
copy over native query and native mutation configurations
hallettj Dec 5, 2024
1911157
update engine
hallettj Dec 5, 2024
31208fc
forgot a relationship
hallettj Dec 5, 2024
5864e29
fixes and updates for integration tests
hallettj Dec 5, 2024
70f64d6
disable test that distributes comparison over array
hallettj Dec 5, 2024
b32a44b
fix lint errors
hallettj Dec 5, 2024
fb7dc7c
propagate aggregate field path to query plan; use path looking up def…
hallettj Dec 3, 2024
6190ea9
update aggregate query generation to use field paths
hallettj Dec 3, 2024
26877cd
use $ne: null instead of $nor: [$eq: null]
hallettj Dec 3, 2024
d3fc0a9
using the top type is more honest
hallettj Dec 3, 2024
2c92981
enable query.nested_fields.aggregates capability
hallettj Dec 5, 2024
2be2b7d
forgot to remove an expression negation
hallettj Dec 5, 2024
677436c
whoops - the supergraph .env file wasn't in version control
hallettj Dec 5, 2024
308e841
switch engine port to 3280 in arion config to match docker-compose co…
hallettj Dec 5, 2024
cde8d86
configure nested field aggregate metadata by hand
hallettj Dec 5, 2024
ec35930
handle null results from aggregates
hallettj Dec 5, 2024
d29fe8a
update metadata, remove custom `count` aggregate
hallettj Dec 6, 2024
c7966f1
Revert "update metadata, remove custom `count` aggregate"
hallettj Dec 6, 2024
f69752a
aggregate results are nullable - except count
hallettj Dec 6, 2024
a358128
tests for everything I've just done
hallettj Dec 6, 2024
0a672ba
Revert "switch engine port to 3280 in arion config to match docker-co…
hallettj Dec 6, 2024
0662d3d
add .env file to version control
hallettj Dec 6, 2024
17a7cfd
Merge branch 'main' into jesse/recreate-supergraph-fixture
hallettj Dec 6, 2024
183e879
Merge branch 'jesse/recreate-supergraph-fixture' into jessehallett/en…
hallettj Dec 6, 2024
e6ee606
update changelog
hallettj Dec 6, 2024
7a599e6
Merge branch 'main' into jessehallett/eng-1024-nested-field-aggregates
hallettj Dec 10, 2024
eb89870
remove double ifNull-wrapping on count results, update tests
hallettj Dec 11, 2024
d45a85a
Merge branch 'main' into jessehallett/eng-1024-nested-field-aggregates
hallettj Dec 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading