fix: generate relationship definition for nested selection (#855)

### What

Previously, while generating relationship definitions for NDC, we would
ignore columns with nested selection.

This PR fixes that.

Closes https://hasurahq.atlassian.net/browse/V3ENGINE-247

### How

While matching on `FieldSelection::Column`, don't ignore it. Check if it
contains nested selection, if it does, call
`collect_relationships_from_nested_selection`

V3_GIT_ORIGIN_REV_ID: 9db94744d8e2d35f8430bded07209ef519175205
This commit is contained in:
Anon Ray 2024-07-23 13:41:47 +05:30 committed by hasura-bot
parent a6dbc7bb1d
commit 9eaabd116f
25 changed files with 516 additions and 4 deletions

View File

@ -6,6 +6,10 @@
### Fixed ### Fixed
- Fixes a bug where relationships within nested columns would throw an internal
error. While generating NDC relationship definitions, engine would ignore
columns with nested selection.
- Renamed the `ArgumentPreset` for data connectors to - Renamed the `ArgumentPreset` for data connectors to
`DataConnectorArgumentPreset` to avoid ambiguity in generated JSONSchema. `DataConnectorArgumentPreset` to avoid ambiguity in generated JSONSchema.
@ -56,9 +60,9 @@ your GraphQL queries more versatile and powerful.
- Build-time check to ensure boolean expressions cannot be built over nested - Build-time check to ensure boolean expressions cannot be built over nested
array fields until these are supported. array fields until these are supported.
- Fixed a bug where command targeted relationships were not using the Open DD - Fixed a bug where command targeted relationships were not using the OpenDD
argument name instead of the data connector's argument name when querying the argument name instead of the data connector's argument name when querying the
data connector data connector.
## [v2024.07.10] ## [v2024.07.10]

View File

@ -1,3 +1,4 @@
{"id":0,"name":"Peter","movie_id":2,"favourite_author_id":1}
{"id":1,"name":"Leonardo DiCaprio","movie_id":1,"favourite_author_id":1} {"id":1,"name":"Leonardo DiCaprio","movie_id":1,"favourite_author_id":1}
{"id":2,"name":"Kate Winslet","movie_id":1,"favourite_author_id":2} {"id":2,"name":"Kate Winslet","movie_id":1,"favourite_author_id":2}
{"id":3,"name":"Irfan Khan","movie_id":2,"favourite_author_id":1} {"id":3,"name":"Irfan Khan","movie_id":2,"favourite_author_id":1}

View File

@ -2,6 +2,11 @@
{ {
"data": { "data": {
"getActorsByMovieIdBounds": [ "getActorsByMovieIdBounds": [
{
"actor_id": 0,
"movie_id": 2,
"name": "Peter"
},
{ {
"actor_id": 3, "actor_id": 3,
"movie_id": 2, "movie_id": 2,

View File

@ -18,6 +18,11 @@
{ {
"data": { "data": {
"ActorsByMovieMany": [ "ActorsByMovieMany": [
{
"actor_id": 0,
"movie_id": 2,
"name": "Peter"
},
{ {
"actor_id": 3, "actor_id": 3,
"movie_id": 2, "movie_id": 2,

View File

@ -2,6 +2,11 @@
{ {
"data": { "data": {
"getActorsByMovieIdBounds": [ "getActorsByMovieIdBounds": [
{
"actor_id": 0,
"movie_id": 2,
"name": "Peter"
},
{ {
"actor_id": 3, "actor_id": 3,
"movie_id": 2, "movie_id": 2,
@ -28,6 +33,11 @@
{ {
"data": { "data": {
"getActorsByMovieIdBounds": [ "getActorsByMovieIdBounds": [
{
"actor_id": 0,
"movie_id": 2,
"name": "Peter"
},
{ {
"actor_id": 3, "actor_id": 3,
"movie_id": 2, "movie_id": 2,

View File

@ -2,6 +2,11 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"actor_id": 0,
"name": "Peter",
"movie_id": 2
},
{ {
"actor_id": 1, "actor_id": 1,
"name": "Leonardo DiCaprio", "name": "Leonardo DiCaprio",
@ -43,6 +48,11 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"actor_id": 0,
"name": "Peter",
"movie_id": 2
},
{ {
"actor_id": 1, "actor_id": 1,
"name": "Leonardo DiCaprio", "name": "Leonardo DiCaprio",

View File

@ -2,6 +2,11 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"actor_id": 0,
"name": "Peter",
"movie_id": 2
},
{ {
"actor_id": 1, "actor_id": 1,
"name": "Leonardo DiCaprio", "name": "Leonardo DiCaprio",

View File

@ -2,6 +2,11 @@
{ {
"data": { "data": {
"uppercaseAllActorNames": [ "uppercaseAllActorNames": [
{
"actor_id": 0,
"movie_id": 2,
"name": "PETER"
},
{ {
"actor_id": 1, "actor_id": 1,
"movie_id": 1, "movie_id": 1,
@ -43,6 +48,11 @@
{ {
"data": { "data": {
"uppercaseAllActorNames": [ "uppercaseAllActorNames": [
{
"actor_id": 0,
"movie_id": 2,
"name": "PETER"
},
{ {
"actor_id": 1, "actor_id": 1,
"movie_id": 1, "movie_id": 1,

View File

@ -2,6 +2,11 @@
{ {
"data": { "data": {
"uppercaseAllActorNames": [ "uppercaseAllActorNames": [
{
"actor_id": 0,
"movie_id": 2,
"name": "PETER"
},
{ {
"actor_id": 1, "actor_id": 1,
"movie_id": 1, "movie_id": 1,

View File

@ -2,6 +2,7 @@
{ {
"data": { "data": {
"uppercaseAllActorNamesReturnNames": [ "uppercaseAllActorNamesReturnNames": [
"PETER",
"LEONARDO DICAPRIO", "LEONARDO DICAPRIO",
"KATE WINSLET", "KATE WINSLET",
"IRFAN KHAN", "IRFAN KHAN",
@ -15,6 +16,7 @@
{ {
"data": { "data": {
"uppercaseAllActorNamesReturnNames": [ "uppercaseAllActorNamesReturnNames": [
"PETER",
"LEONARDO DICAPRIO", "LEONARDO DICAPRIO",
"KATE WINSLET", "KATE WINSLET",
"IRFAN KHAN", "IRFAN KHAN",

View File

@ -0,0 +1,93 @@
{
"data": {
"InstitutionMany": [
{
"id": 1,
"location": {
"city": "London",
"campuses": [
"Mile End",
"Whitechapel",
"Charterhouse Square",
"West Smithfield"
]
},
"location_country": {
"country": "UK"
},
"staff": [
{
"last_name": "Landin",
"specialities": ["Computer Science", "Education"],
"actor": {
"name": "Peter"
}
}
],
"staff_first_name": [
{
"first_name": "Peter"
}
],
"departments": [
"Humanities and Social Sciences",
"Science and Engineering",
"Medicine and Dentistry"
]
},
{
"id": 2,
"location": {
"city": "Gothenburg",
"campuses": ["Johanneberg", "Lindholmen"]
},
"location_country": {
"country": "Sweden"
},
"staff": [
{
"last_name": "Hughes",
"specialities": [
"Computer Science",
"Functional Programming",
"Software Testing"
],
"actor": null
},
{
"last_name": "Claessen",
"specialities": [
"Computer Science",
"Functional Programming",
"Automated Reasoning"
],
"actor": null
}
],
"staff_first_name": [
{
"first_name": "John"
},
{
"first_name": "Koen"
}
],
"departments": [
"Architecture and Civil Engineering",
"Computer Science and Engineering",
"Electrical Engineering",
"Physics",
"Industrial and Materials Science"
]
},
{
"id": 3,
"location": null,
"location_country": null,
"staff": null,
"staff_first_name": null,
"departments": ["nothing", null]
}
]
}
}

View File

@ -0,0 +1,171 @@
{
"version": "v2",
"subgraphs": [
{
"name": "default",
"objects": [
{
"kind": "ObjectType",
"version": "v1",
"definition": {
"name": "actor",
"fields": [
{
"name": "actor_id",
"type": "Int!"
},
{
"name": "name",
"type": "String!"
},
{
"name": "movie_id",
"type": "Int!"
}
],
"graphql": {
"typeName": "Actor"
},
"dataConnectorTypeMapping": [
{
"dataConnectorName": "custom",
"dataConnectorObjectType": "actor",
"fieldMapping": {
"actor_id": {
"column": {
"name": "id"
}
},
"name": {
"column": {
"name": "name"
}
},
"movie_id": {
"column": {
"name": "movie_id"
}
}
}
}
]
}
},
{
"kind": "TypePermissions",
"version": "v1",
"definition": {
"typeName": "actor",
"permissions": [
{
"role": "admin",
"output": {
"allowedFields": ["actor_id", "name", "movie_id"]
}
},
{
"role": "user",
"output": {
"allowedFields": ["actor_id", "name", "movie_id"]
}
}
]
}
},
{
"kind": "Model",
"version": "v1",
"definition": {
"name": "Actors",
"objectType": "actor",
"source": {
"dataConnectorName": "custom",
"collection": "actors"
},
"graphql": {
"selectUniques": [],
"selectMany": {
"queryRootField": "ActorMany"
}
},
"orderableFields": [
{
"fieldName": "actor_id",
"orderByDirections": {
"enableAll": true
}
},
{
"fieldName": "name",
"orderByDirections": {
"enableAll": true
}
},
{
"fieldName": "movie_id",
"orderByDirections": {
"enableAll": true
}
}
]
}
},
{
"kind": "ModelPermissions",
"version": "v1",
"definition": {
"modelName": "Actors",
"permissions": [
{
"role": "admin",
"select": {
"filter": null
}
},
{
"role": "user",
"select": {
"filter": null
}
}
]
}
},
{
"kind": "Relationship",
"version": "v1",
"definition": {
"name": "actor",
"sourceType": "staff_member",
"target": {
"model": {
"name": "Actors",
"subgraph": "default",
"relationshipType": "Object"
}
},
"mapping": [
{
"source": {
"fieldPath": [
{
"fieldName": "first_name"
}
]
},
"target": {
"modelField": [
{
"fieldName": "name"
}
]
}
}
],
"description": "Random mapping of a staff member to an actor"
}
}
]
}
]
}

View File

@ -0,0 +1,24 @@
query RelationshipInNestedObject {
InstitutionMany {
id
location {
city
campuses
}
location_country: location {
country
}
staff {
last_name
specialities
# nested object -> relationship
actor {
name
}
}
staff_first_name: staff {
first_name
}
departments
}
}

View File

@ -0,0 +1,3 @@
{
"x-hasura-role": "admin"
}

View File

@ -2,6 +2,15 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"MovieFromActor": {
"movie_id": 2,
"rating": 5,
"title": "Slumdog Millionaire"
},
"actor_id": 0,
"name": "Peter"
},
{ {
"MovieFromActor": { "MovieFromActor": {
"movie_id": 1, "movie_id": 1,

View File

@ -2,6 +2,15 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"Movies": {
"movie_id": 2,
"rating": 5,
"title": "Slumdog Millionaire"
},
"actor_id": 0,
"name": "Peter"
},
{ {
"Movies": { "Movies": {
"movie_id": 1, "movie_id": 1,
@ -71,6 +80,11 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"Movies": null,
"actor_id": 0,
"name": "Peter"
},
{ {
"Movies": { "Movies": {
"movie_id": 1, "movie_id": 1,
@ -120,6 +134,11 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"Movies": null,
"actor_id": 0,
"name": "Peter"
},
{ {
"Movies": null, "Movies": null,
"actor_id": 1, "actor_id": 1,
@ -161,6 +180,11 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"Movies": null,
"actor_id": 0,
"name": "Peter"
},
{ {
"Movies": null, "Movies": null,
"actor_id": 1, "actor_id": 1,

View File

@ -2,6 +2,15 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"Movies": {
"movie_id": 2,
"rating": 5,
"title": "Slumdog Millionaire"
},
"actor_id": 0,
"name": "Peter"
},
{ {
"Movies": { "Movies": {
"movie_id": 1, "movie_id": 1,
@ -71,6 +80,11 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"Movies": null,
"actor_id": 0,
"name": "Peter"
},
{ {
"Movies": { "Movies": {
"movie_id": 1, "movie_id": 1,
@ -120,6 +134,11 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"Movies": null,
"actor_id": 0,
"name": "Peter"
},
{ {
"Movies": null, "Movies": null,
"actor_id": 1, "actor_id": 1,
@ -161,6 +180,11 @@
{ {
"data": { "data": {
"getAllActors": [ "getAllActors": [
{
"Movies": null,
"actor_id": 0,
"name": "Peter"
},
{ {
"Movies": null, "Movies": null,
"actor_id": 1, "actor_id": 1,

View File

@ -2,6 +2,14 @@
{ {
"data": { "data": {
"ActorMany": [ "ActorMany": [
{
"MovieFromCommand": {
"movie_id": 2,
"rating": 5,
"title": "Slumdog Millionaire"
},
"name": "Peter"
},
{ {
"MovieFromCommand": { "MovieFromCommand": {
"movie_id": 1, "movie_id": 1,

View File

@ -24,6 +24,13 @@
{ {
"title": "Slumdog Millionaire", "title": "Slumdog Millionaire",
"Actors": [ "Actors": [
{
"name": "Peter",
"MovieFromCommand": {
"title": "Slumdog Millionaire",
"rating": 5
}
},
{ {
"name": "Irfan Khan", "name": "Irfan Khan",
"MovieFromCommand": { "MovieFromCommand": {

View File

@ -45,6 +45,9 @@
"Movie": { "Movie": {
"title": "Slumdog Millionaire", "title": "Slumdog Millionaire",
"Actors": [ "Actors": [
{
"name": "Peter"
},
{ {
"name": "Irfan Khan" "name": "Irfan Khan"
} }
@ -120,6 +123,9 @@
"Movie": { "Movie": {
"title": "Slumdog Millionaire", "title": "Slumdog Millionaire",
"Actors": [ "Actors": [
{
"name": "Peter"
},
{ {
"name": "Irfan Khan" "name": "Irfan Khan"
} }

View File

@ -45,6 +45,9 @@
"Movie": { "Movie": {
"title": "Slumdog Millionaire", "title": "Slumdog Millionaire",
"Actors": [ "Actors": [
{
"name": "Peter"
},
{ {
"name": "Irfan Khan" "name": "Irfan Khan"
} }
@ -120,6 +123,9 @@
"Movie": { "Movie": {
"title": "Slumdog Millionaire", "title": "Slumdog Millionaire",
"Actors": [ "Actors": [
{
"name": "Peter"
},
{ {
"name": "Irfan Khan" "name": "Irfan Khan"
} }

View File

@ -42,6 +42,18 @@
"title": "Slumdog Millionaire", "title": "Slumdog Millionaire",
"rating": 5, "rating": 5,
"Actors": [ "Actors": [
{
"name": "Peter",
"Movie": {
"title": "Slumdog Millionaire",
"Analytics": [
{
"num_users_faved": 4,
"num_views_day": 13
}
]
}
},
{ {
"name": "Irfan Khan", "name": "Irfan Khan",
"Movie": { "Movie": {
@ -136,6 +148,18 @@
"title": "Slumdog Millionaire", "title": "Slumdog Millionaire",
"rating": 5, "rating": 5,
"Actors": [ "Actors": [
{
"name": "Peter",
"Movie": {
"title": "Slumdog Millionaire",
"Analytics": [
{
"num_users_faved": 4,
"num_views_day": 13
}
]
}
},
{ {
"name": "Irfan Khan", "name": "Irfan Khan",
"Movie": { "Movie": {
@ -166,6 +190,13 @@
"title": "Slumdog Millionaire", "title": "Slumdog Millionaire",
"rating": 5, "rating": 5,
"Actors": [ "Actors": [
{
"name": "Peter",
"Movie": {
"title": "Slumdog Millionaire",
"Analytics": []
}
},
{ {
"name": "Irfan Khan", "name": "Irfan Khan",
"Movie": { "Movie": {

View File

@ -30,6 +30,15 @@
"title": "Slumdog Millionaire", "title": "Slumdog Millionaire",
"rating": 5, "rating": 5,
"Actors": [ "Actors": [
{
"name": "Peter",
"favourite_author_id": 1,
"FavouriteAuthor": {
"author_id": 1,
"first_name": "Peter",
"last_name": "Landin"
}
},
{ {
"name": "Irfan Khan", "name": "Irfan Khan",
"favourite_author_id": 1, "favourite_author_id": 1,
@ -105,6 +114,15 @@
"title": "Slumdog Millionaire", "title": "Slumdog Millionaire",
"rating": 5, "rating": 5,
"Actors": [ "Actors": [
{
"name": "Peter",
"favourite_author_id": 1,
"FavouriteAuthor": {
"author_id": 1,
"first_name": "Peter",
"last_name": "Landin"
}
},
{ {
"name": "Irfan Khan", "name": "Irfan Khan",
"favourite_author_id": 1, "favourite_author_id": 1,
@ -126,6 +144,15 @@
"title": "Slumdog Millionaire", "title": "Slumdog Millionaire",
"rating": 5, "rating": 5,
"Actors": [ "Actors": [
{
"name": "Peter",
"favourite_author_id": 1,
"FavouriteAuthor": {
"author_id": 1,
"first_name": "Peter",
"last_name": "Landin"
}
},
{ {
"name": "Irfan Khan", "name": "Irfan Khan",
"favourite_author_id": 1, "favourite_author_id": 1,

View File

@ -116,6 +116,17 @@ fn test_model_select_many_nested_select_no_explicit_type_mapping() -> anyhow::Re
common::test_execution_expectation_legacy(test_path_string, &[common_metadata_path_string]) common::test_execution_expectation_legacy(test_path_string, &[common_metadata_path_string])
} }
// Same test as the nested selection
#[test]
fn test_model_select_many_nested_select_with_relationship() -> anyhow::Result<()> {
let test_path_string = "execute/models/select_many/nested_select/relationship";
let common_metadata_paths = [
"execute/common_metadata/custom_connector_schema.json",
"execute/models/select_many/nested_select/metadata.json",
];
common::test_execution_expectation_legacy(test_path_string, &common_metadata_paths)
}
// nested selection tests, using Postgres // nested selection tests, using Postgres
#[test] #[test]
fn test_model_select_many_nested_select_postgres() -> anyhow::Result<()> { fn test_model_select_many_nested_select_postgres() -> anyhow::Result<()> {

View File

@ -47,10 +47,21 @@ pub(crate) fn collect_relationships(
)?; )?;
} }
} }
FieldSelection::Column { .. } FieldSelection::Column {
arguments: _,
column: _,
nested_selection,
} => {
if let Some(nested_selection) = &nested_selection {
selection_set::collect_relationships_from_nested_selection(
nested_selection,
relationships,
)?;
}
}
// we ignore remote relationships as we are generating relationship // we ignore remote relationships as we are generating relationship
// definition for one data connector // definition for one data connector
| FieldSelection::ModelRelationshipRemote { .. } FieldSelection::ModelRelationshipRemote { .. }
| FieldSelection::CommandRelationshipRemote { .. } => (), | FieldSelection::CommandRelationshipRemote { .. } => (),
}; };
} }