Skip to content

Commit 5fe65f8

Browse files
Aniruddh25seantleonard
Aniruddh25
and
seantleonard
authored
Make relationship fields in referenced entities nullable (#1958)
## Why make this change? - Closes #1747 - Records in referenced entities may not always be related to the referencing entity. E.g. in a `book-websiteplacement` relationship, not all `books` (referenced entity) may have a `websiteplacement` (referencing) yet. This change is to ensure we get all records of the referenced entity including the ones that DON'T have any relationships. ## What is this change? - Previously, we used to rely on the nullability of the referenced fields to determine whether the relationship field in the referenced entity should be nullable or not. While this is applicable when we are considering the relationship fields of a referencing entity, it is actually restricting when used for a referenced entity. - For example, the referencing entity (BookWebsitePlacement) should have a nullable `books` (relationship field) based on whether the foreign key `book_id` in `book_website_placements` is nullable or not -> indicating whether a website placement MUST have a book or not. - On the other hand, the referenced entity `books` should always have a NULLABLE `websiteplacement` relationship field in order to include those books that don't have any `websiteplacement` published yet. Relying on the nullability of the `id` - the referenced field in the book->book_website_placement foreign key would make the relationship NON-NULLABLE but this restricts inclusion of those books that don't have any website placements hence the `error: "Cannot return null for non-nullable field"`. We need to make the relationship field nullable in such cases. - The source entity could be both the referencing and referenced entity in case of missing foreign keys in the db or self referencing relationships. - Use the nullability of referencing columns to determine the nullability of the relationship field only if 1. there is exactly one relationship where source is the referencing entity. DAB doesn't support multiple relationships at the moment. and 2. when the source is not a referenced entity in any of the relationships. - Identifies the scenario where the relationship field is from the referenced entity and always sets its nullability to `true`. ## How was this tested? - [X] Integration Tests - existing data already had this bug, needed to modify the query to expose it. - Ran the query in #1747 to verify this fix solved that issue. ## Sample Request(s) ```GraphQL { books { items { id title websiteplacement { price } } } } ``` BEFORE: ![image](https://github.com/Azure/data-api-builder/assets/3513779/17602247-db9b-4b61-9e42-cccf527306b5) AFTER FIX: There are no more errors and note that we now actually return `null` as the relationship field value(here, `websiteplacement`) when querying the referenced entity (here, `book`) which doesn't have any relationship with the referencing entity. ![image](https://github.com/Azure/data-api-builder/assets/3513779/eec87cc0-8341-44bd-98d9-3bcf7fd4bc40) ## NOTE The issue is only exposed in a 1:1 relationship. This is because the only other scenario for querying a referenced entity is a 1:many relationship. And for records in the referenced entity(e.g. publisher) which are NOT related to the referencing entity(e.g. book), we explicitly check for nulls and return an empty array. See here: https://github.com/Azure/data-api-builder/blob/dacf3bccbe0a36da51776bf4afd1b4a0d4348ff4/src/Core/Resolvers/SqlQueryEngine.cs#L123 E.g. ```GraphQL Query for 1:many relationship { publishers { items { id name books { items { title } } } } } ``` Expected Response ![image](https://github.com/Azure/data-api-builder/assets/3513779/556cee8f-80d4-4fd3-b365-9046ec31326e) NOTE: When DAB supports multiple relationships, nullability of each relationship field should be determined based on foreign keys associated with each relationship. --------- Co-authored-by: Sean Leonard <[email protected]>
1 parent e681aa5 commit 5fe65f8

File tree

7 files changed

+160
-190
lines changed

7 files changed

+160
-190
lines changed

‎src/Service.GraphQLBuilder/Sql/SchemaConverter.cs‎

Lines changed: 82 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -121,42 +121,7 @@ public static ObjectTypeDefinitionNode FromDatabaseObject(
121121
string targetEntityName = relationship.TargetEntity.Split('.').Last();
122122
Entity referencedEntity = entities[targetEntityName];
123123

124-
bool isNullableRelationship = false;
125-
126-
if (// Retrieve all the relationship information for the source entity which is backed by this table definition
127-
sourceDefinition.SourceEntityRelationshipMap.TryGetValue(entityName, out RelationshipMetadata? relationshipInfo)
128-
&&
129-
// From the relationship information, obtain the foreign key definition for the given target entity
130-
relationshipInfo.TargetEntityToFkDefinitionMap.TryGetValue(targetEntityName,
131-
out List<ForeignKeyDefinition>? listOfForeignKeys))
132-
{
133-
ForeignKeyDefinition? foreignKeyInfo = listOfForeignKeys.FirstOrDefault();
134-
135-
// Determine whether the relationship should be nullable by obtaining the nullability
136-
// of the referencing(if source entity is the referencing object in the pair)
137-
// or referenced columns (if source entity is the referenced object in the pair).
138-
if (foreignKeyInfo is not null)
139-
{
140-
RelationShipPair pair = foreignKeyInfo.Pair;
141-
// The given entity may be the referencing or referenced database object in the foreign key
142-
// relationship. To determine this, compare with the entity's database object.
143-
if (pair.ReferencingDbTable.Equals(databaseObject))
144-
{
145-
isNullableRelationship = sourceDefinition.IsAnyColumnNullable(foreignKeyInfo.ReferencingColumns);
146-
}
147-
else
148-
{
149-
isNullableRelationship = sourceDefinition.IsAnyColumnNullable(foreignKeyInfo.ReferencedColumns);
150-
}
151-
}
152-
else
153-
{
154-
throw new DataApiBuilderException(
155-
message: $"No relationship exists between {entityName} and {targetEntityName}",
156-
statusCode: HttpStatusCode.InternalServerError,
157-
subStatusCode: DataApiBuilderException.SubStatusCodes.GraphQLMapping);
158-
}
159-
}
124+
bool isNullableRelationship = FindNullabilityOfRelationship(entityName, databaseObject, targetEntityName);
160125

161126
INullableTypeNode targetField = relationship.Cardinality switch
162127
{
@@ -276,5 +241,86 @@ public static IValueNode CreateValueNodeFromDbObjectMetadata(object metadataValu
276241

277242
return arg;
278243
}
244+
245+
/// <summary>
246+
/// Given the source entity name, its underlying database object and the targetEntityName,
247+
/// finds if the relationship field corresponding to the target should be nullable
248+
/// based on whether the source is the referencing or referenced object or both.
249+
/// </summary>
250+
/// <exception cref="DataApiBuilderException">Raised no relationship exists between the source and target
251+
/// entities.</exception>
252+
private static bool FindNullabilityOfRelationship(
253+
string entityName,
254+
DatabaseObject databaseObject,
255+
string targetEntityName)
256+
{
257+
bool isNullableRelationship = false;
258+
SourceDefinition sourceDefinition = databaseObject.SourceDefinition;
259+
if (// Retrieve all the relationship information for the source entity which is backed by this table definition
260+
sourceDefinition.SourceEntityRelationshipMap.TryGetValue(entityName, out RelationshipMetadata? relationshipInfo)
261+
&&
262+
// From the relationship information, obtain the foreign key definition for the given target entity
263+
relationshipInfo.TargetEntityToFkDefinitionMap.TryGetValue(targetEntityName,
264+
out List<ForeignKeyDefinition>? listOfForeignKeys))
265+
{
266+
// DAB optimistically adds entries to 'listOfForeignKeys' representing each relationship direction
267+
// between a pair of entities when 1:1 or many:1 relationships are defined in the runtime config.
268+
// Entries which don't have a matching corresponding foreign key in the database
269+
// will have 0 referencing/referenced columns. So, we need to filter out these
270+
// invalid entries. Non-zero referenced columns indicate valid matching foreign key definition in the
271+
// database and hence only those can be used to determine the directionality.
272+
273+
// Find the foreignkeys in which the source entity is the referencing object.
274+
IEnumerable<ForeignKeyDefinition> referencingForeignKeyInfo =
275+
listOfForeignKeys.Where(fk =>
276+
fk.ReferencingColumns.Count > 0
277+
&& fk.ReferencedColumns.Count > 0
278+
&& fk.Pair.ReferencingDbTable.Equals(databaseObject));
279+
280+
// Find the foreignkeys in which the source entity is the referenced object.
281+
IEnumerable<ForeignKeyDefinition> referencedForeignKeyInfo =
282+
listOfForeignKeys.Where(fk =>
283+
fk.ReferencingColumns.Count > 0
284+
&& fk.ReferencedColumns.Count > 0
285+
&& fk.Pair.ReferencedDbTable.Equals(databaseObject));
286+
287+
// The source entity should at least be a referencing or referenced db object or both
288+
// in the foreign key relationship.
289+
if (referencingForeignKeyInfo.Count() > 0 || referencedForeignKeyInfo.Count() > 0)
290+
{
291+
// The source entity could be both the referencing and referenced entity
292+
// in case of missing foreign keys in the db or self referencing relationships.
293+
// Use the nullability of referencing columns to determine
294+
// the nullability of the relationship field only if
295+
// 1. there is exactly one relationship where source is the referencing entity.
296+
// DAB doesn't support multiple relationships at the moment.
297+
// and
298+
// 2. when the source is not a referenced entity in any of the relationships.
299+
if (referencingForeignKeyInfo.Count() == 1 && referencedForeignKeyInfo.Count() == 0)
300+
{
301+
ForeignKeyDefinition foreignKeyInfo = referencingForeignKeyInfo.First();
302+
isNullableRelationship = sourceDefinition.IsAnyColumnNullable(foreignKeyInfo.ReferencingColumns);
303+
}
304+
else
305+
{
306+
// a record of the "referenced" entity may or may not have a relationship with
307+
// any other record of the referencing entity in the database
308+
// (irrespective of nullability of the referenced columns)
309+
// Setting the relationship field to nullable ensures even those records
310+
// that are not related are considered while querying.
311+
isNullableRelationship = true;
312+
}
313+
}
314+
else
315+
{
316+
throw new DataApiBuilderException(
317+
message: $"No relationship exists between {entityName} and {targetEntityName}",
318+
statusCode: HttpStatusCode.InternalServerError,
319+
subStatusCode: DataApiBuilderException.SubStatusCodes.GraphQLMapping);
320+
}
321+
}
322+
323+
return isNullableRelationship;
324+
}
279325
}
280326
}

‎src/Service.Tests/GraphQLRequestExecutor.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using System;
45
using System.Collections.Generic;
56
using System.Net.Http;
67
using System.Net.Http.Json;
@@ -67,6 +68,7 @@ public static async Task<JsonElement> PostGraphQLRequestAsync(
6768
if (graphQLResult.TryGetProperty("errors", out JsonElement errors))
6869
{
6970
// to validate expected errors and error message
71+
Console.WriteLine($"GraphQL error: {errors}");
7072
return errors;
7173
}
7274

‎src/Service.Tests/SqlTests/GraphQLQueryTests/DwSqlGraphQLQueryTests.cs‎

Lines changed: 25 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -68,46 +68,32 @@ ORDER BY [__column1] asc
6868
[TestMethod]
6969
public async Task OneToOneJoinQuery()
7070
{
71-
string msSqlQuery = @"
72-
SELECT
73-
TOP 1 [table0].[id] AS [id],
74-
JSON_QUERY ([table1_subq].[data]) AS [websiteplacement]
75-
FROM
76-
[books] AS [table0]
77-
OUTER APPLY (
78-
SELECT
79-
TOP 1 [table1].[id] AS [id],
80-
[table1].[price] AS [price],
81-
JSON_QUERY ([table2_subq].[data]) AS [books]
82-
FROM
83-
[book_website_placements] AS [table1]
84-
OUTER APPLY (
85-
SELECT
86-
TOP 1 [table2].[id] AS [id]
87-
FROM
88-
[books] AS [table2]
89-
WHERE
90-
[table1].[book_id] = [table2].[id]
91-
ORDER BY
92-
[table2].[id] Asc FOR JSON PATH,
93-
INCLUDE_NULL_VALUES,
94-
WITHOUT_ARRAY_WRAPPER
95-
) AS [table2_subq]([data])
96-
WHERE
97-
[table1].[book_id] = [table0].[id]
98-
ORDER BY
99-
[table1].[id] Asc FOR JSON PATH,
100-
INCLUDE_NULL_VALUES,
101-
WITHOUT_ARRAY_WRAPPER
102-
) AS [table1_subq]([data])
103-
WHERE
104-
[table0].[id] = 1
105-
ORDER BY
106-
[table0].[id] Asc FOR JSON PATH,
107-
INCLUDE_NULL_VALUES,
108-
WITHOUT_ARRAY_WRAPPER";
71+
string dwSqlQuery = @"
72+
SELECT COALESCE('[' + STRING_AGG('{' + N'""id"":' + ISNULL(STRING_ESCAPE(CAST([id] AS NVARCHAR(MAX)), 'json'),
73+
'null') + ',' + N'""title"":' + ISNULL('""' + STRING_ESCAPE([title], 'json') + '""', 'null') + ',' +
74+
N'""websiteplacement"":' + ISNULL([websiteplacement], 'null') +
75+
'}', ', ') + ']', '[]')
76+
FROM (
77+
SELECT TOP 100 [table0].[id] AS [id],
78+
[table0].[title] AS [title],
79+
([table1_subq].[data]) AS [websiteplacement]
80+
FROM [dbo].[books] AS [table0]
81+
OUTER APPLY (
82+
SELECT STRING_AGG('{' + N'""price"":' + ISNULL(STRING_ESCAPE(CAST([price] AS NVARCHAR(MAX)), 'json'),
83+
'null') + '}', ', ')
84+
FROM (
85+
SELECT TOP 1 [table1].[price] AS [price]
86+
FROM [dbo].[book_website_placements] AS [table1]
87+
WHERE [table0].[id] = [table1].[book_id]
88+
AND [table1].[book_id] = [table0].[id]
89+
ORDER BY [table1].[id] ASC
90+
) AS [table1]
91+
) AS [table1_subq]([data])
92+
WHERE 1 = 1
93+
ORDER BY [table0].[id] ASC
94+
) AS [table0]";
10995

110-
await OneToOneJoinQuery(msSqlQuery);
96+
await OneToOneJoinQuery(dwSqlQuery);
11197
}
11298

11399
/// <summary>

‎src/Service.Tests/SqlTests/GraphQLQueryTests/GraphQLQueryTestBase.cs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -556,24 +556,23 @@ public async Task QueryAgainstSPWithOnlyTypenameInSelectionSet(string dbQuery)
556556
[TestMethod]
557557
public async Task OneToOneJoinQuery(string dbQuery)
558558
{
559-
string graphQLQueryName = "book_by_pk";
559+
string graphQLQueryName = "books";
560560
string graphQLQuery = @"query {
561-
book_by_pk(id: 1) {
562-
id
563-
websiteplacement {
561+
books {
562+
items {
564563
id
564+
title
565+
websiteplacement {
565566
price
566-
books {
567-
id
568-
}
569567
}
570568
}
569+
}
571570
}";
572571

573572
JsonElement actual = await base.ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false);
574573
string expected = await GetDatabaseResultAsync(dbQuery);
575574

576-
SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString());
575+
SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.GetProperty("items").ToString());
577576
}
578577

579578
/// <summary>

‎src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs‎

Lines changed: 17 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -83,43 +83,23 @@ ORDER BY [__column1] asc
8383
public async Task OneToOneJoinQuery()
8484
{
8585
string msSqlQuery = @"
86-
SELECT
87-
TOP 1 [table0].[id] AS [id],
88-
JSON_QUERY ([table1_subq].[data]) AS [websiteplacement]
89-
FROM
90-
[books] AS [table0]
91-
OUTER APPLY (
92-
SELECT
93-
TOP 1 [table1].[id] AS [id],
94-
[table1].[price] AS [price],
95-
JSON_QUERY ([table2_subq].[data]) AS [books]
96-
FROM
97-
[book_website_placements] AS [table1]
98-
OUTER APPLY (
99-
SELECT
100-
TOP 1 [table2].[id] AS [id]
101-
FROM
102-
[books] AS [table2]
103-
WHERE
104-
[table1].[book_id] = [table2].[id]
105-
ORDER BY
106-
[table2].[id] Asc FOR JSON PATH,
107-
INCLUDE_NULL_VALUES,
108-
WITHOUT_ARRAY_WRAPPER
109-
) AS [table2_subq]([data])
110-
WHERE
111-
[table1].[book_id] = [table0].[id]
112-
ORDER BY
113-
[table1].[id] Asc FOR JSON PATH,
114-
INCLUDE_NULL_VALUES,
115-
WITHOUT_ARRAY_WRAPPER
116-
) AS [table1_subq]([data])
117-
WHERE
118-
[table0].[id] = 1
119-
ORDER BY
120-
[table0].[id] Asc FOR JSON PATH,
121-
INCLUDE_NULL_VALUES,
122-
WITHOUT_ARRAY_WRAPPER";
86+
SELECT TOP 100 [table0].[id] AS [id]
87+
,[table0].[title] AS [title]
88+
,JSON_QUERY([table1_subq].[data]) AS [websiteplacement]
89+
FROM [dbo].[books] AS [table0]
90+
OUTER APPLY (
91+
SELECT TOP 1 [table1].[price] AS [price]
92+
FROM [dbo].[book_website_placements] AS [table1]
93+
WHERE [table1].[book_id] = [table0].[id]
94+
ORDER BY [table1].[id] ASC
95+
FOR JSON PATH
96+
,INCLUDE_NULL_VALUES
97+
,WITHOUT_ARRAY_WRAPPER
98+
) AS [table1_subq]([data])
99+
WHERE 1 = 1
100+
ORDER BY [table0].[id] ASC
101+
FOR JSON PATH
102+
,INCLUDE_NULL_VALUES";
123103

124104
await OneToOneJoinQuery(msSqlQuery);
125105
}

‎src/Service.Tests/SqlTests/GraphQLQueryTests/MySqlGraphQLQueryTests.cs‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -107,30 +107,22 @@ ORDER BY `table0`.`id` asc
107107
public async Task OneToOneJoinQuery()
108108
{
109109
string mySqlQuery = @"
110-
SELECT JSON_OBJECT('id', `subq11`.`id`, 'websiteplacement', `subq11`.`websiteplacement`)
111-
AS `data`
110+
SELECT COALESCE(JSON_ARRAYAGG(JSON_OBJECT('id', `subq7`.`id`, 'title', `subq7`.`title`, 'websiteplacement',
111+
`subq7`.`websiteplacement`)), JSON_ARRAY()) AS `data`
112112
FROM (
113113
SELECT `table0`.`id` AS `id`,
114+
`table0`.`title` AS `title`,
114115
`table1_subq`.`data` AS `websiteplacement`
115116
FROM `books` AS `table0`
116-
LEFT OUTER JOIN LATERAL(SELECT JSON_OBJECT('id', `subq10`.`id`, 'price', `subq10`.`price`, 'books',
117-
`subq10`.`books`) AS `data` FROM (
118-
SELECT `table1`.`id` AS `id`,
119-
`table1`.`price` AS `price`,
120-
`table2_subq`.`data` AS `books`
117+
LEFT OUTER JOIN LATERAL(SELECT JSON_OBJECT('price', `subq6`.`price`) AS `data` FROM (
118+
SELECT `table1`.`price` AS `price`
121119
FROM `book_website_placements` AS `table1`
122-
LEFT OUTER JOIN LATERAL(SELECT JSON_OBJECT('id', `subq9`.`id`) AS `data` FROM (
123-
SELECT `table2`.`id` AS `id`
124-
FROM `books` AS `table2`
125-
WHERE `table1`.`book_id` = `table2`.`id`
126-
ORDER BY `table2`.`id` asc LIMIT 1
127-
) AS `subq9`) AS `table2_subq` ON TRUE
128-
WHERE `table0`.`id` = `table1`.`book_id`
129-
ORDER BY `table1`.`id` asc LIMIT 1
130-
) AS `subq10`) AS `table1_subq` ON TRUE
131-
WHERE `table0`.`id` = 1
132-
ORDER BY `table0`.`id` asc LIMIT 100
133-
) AS `subq11`
120+
WHERE `table1`.`book_id` = `table0`.`id`
121+
ORDER BY `table1`.`id` ASC LIMIT 1
122+
) AS `subq6`) AS `table1_subq` ON TRUE
123+
WHERE 1 = 1
124+
ORDER BY `table0`.`id` ASC LIMIT 100
125+
) AS `subq7`
134126
";
135127

136128
await OneToOneJoinQuery(mySqlQuery);

0 commit comments

Comments
 (0)