-
Notifications
You must be signed in to change notification settings - Fork 279
Multiple Create - Database Query Generation and Selection Set Resolution #1994
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
Multiple Create - Database Query Generation and Selection Set Resolution #1994
Conversation
…ithub.com/Azure/data-api-builder into dev/agarwalayush/AuthZForNestedInsertions
/azp run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comments so far, still have more to review.
src/Core/Resolvers/Sql Query Structures/MultipleCreateStructure.cs
Outdated
Show resolved
Hide resolved
src/Core/Resolvers/Sql Query Structures/MultipleCreateStructure.cs
Outdated
Show resolved
Hide resolved
/azp run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more comments. Still reviewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more feedback. Now on to test files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Massive PR, and looks like good test coverage across various scenarios that are possible. Thank you for pushing through on this. Looking forward to further using this and additional bug bashing to play around with this feature!
.../SqlTests/GraphQLMutationTests/MultipleCreateMutationTests/MultipleCreateMutationTestBase.cs
Outdated
Show resolved
Hide resolved
.../SqlTests/GraphQLMutationTests/MultipleCreateMutationTests/MultipleCreateMutationTestBase.cs
Show resolved
Hide resolved
.../SqlTests/GraphQLMutationTests/MultipleCreateMutationTests/MultipleCreateMutationTestBase.cs
Outdated
Show resolved
Hide resolved
.../SqlTests/GraphQLMutationTests/MultipleCreateMutationTests/MultipleCreateMutationTestBase.cs
Outdated
Show resolved
Hide resolved
.../SqlTests/GraphQLMutationTests/MultipleCreateMutationTests/MultipleCreateMutationTestBase.cs
Outdated
Show resolved
Hide resolved
.../SqlTests/GraphQLMutationTests/MultipleCreateMutationTests/MultipleCreateMutationTestBase.cs
Outdated
Show resolved
Hide resolved
.../SqlTests/GraphQLMutationTests/MultipleCreateMutationTests/MultipleCreateMutationTestBase.cs
Outdated
Show resolved
Hide resolved
.../SqlTests/GraphQLMutationTests/MultipleCreateMutationTests/MultipleCreateMutationTestBase.cs
Outdated
Show resolved
Hide resolved
.../SqlTests/GraphQLMutationTests/MultipleCreateMutationTests/MultipleCreateMutationTestBase.cs
Outdated
Show resolved
Hide resolved
...qlTests/GraphQLMutationTests/MultipleCreateMutationTests/MsSqlMultipleCreateMutationTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feedback is mostly nits but necessary to implement to improve code readability for others' understanding. No logic changes requested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after addressing unresolved feedback. Thanks for completing the puzzle :)
/azp run |
Why make this change?
This PR tackles the a)Database Query Generation and b)Selection Set Resolution components of Multiple Mutation/Multiple Create feature.
What is this change?
1. Enhancing the Hotchocolate input parsing logic
At the moment, DAB supports create mutation operation on a single entity and a single item. The current parsing logic
BaseSqlQueryStructure.GQLMutArgumentToDictParams()
is written with a foundational idea that a) All the fields present in the create mutation input request belong to the top-level entity b) All the fields will be scalar fields.With the multiple create feature, these no longer hold true. Here, are some items that needs to be accounted for by the parsing logic.
a) The fields present in the input request, could correspond to columns of the top-level entity or could be relationship fields
b) Relationship fields are not scalar fields. They could be of Object type or List type depending on the type of relationship between top level entity and the related entity.
c) Since, nesting levels without any limit is supported, a relationship field can further have a combination of scalar fields + relationship fields. This inner relationship field can again be a combination of scalar fields + relationship fields and so on...
d) In addition to point create mutation, a many type mutation operation is also generated. Ex:
createbooks
,createBookmarks_multiple
. These operations let the user create multiple items of the top-level entity. The root input field for this operation will beitems
as opposed toitem
(for a point create mutation operation).New parsing logic:
SqlMutationEngine.GQLMultipleCreateArgumentToDictParams()
Final data structure of the input params after parsing:
Many type Multiple Create:
List<IDictionary<string, object?>>
Point Multiple Create:
IDictionary<string, object?>
2. MultipleCreateStructure - New Wrapper class introduced
MultipleCreateStructure
is used to hold all the relevant information about an entity needed for processing it - determining if there are referencing/referenced entities, determining if there is a linking table insertion necessary along with it, populating the relationship fields in the current entity and linking entity.3. Identifying and handling the implications of a point vs many multiple create operation
Logic for identifying and handling different multiple type mutation operations - Point vs Many multiple create operation is added in
SqlMutationEngine.PerformMultipleCreateOperation()
4. Understanding and processing the parsed mutation input request
Logic added in
SqlMutationEngine.PerformDbInsertOperation()
.This function handles the core logic from understanding the different fields of the parsed input parameters all the way till the successful creation of records in the database. This function is invoked for both point and many type multiple create operations.
The logic in this function at a high-level can be summarized as follows:
create
policy (if defined) are added.5. Introduction of synchronous methods in QueryExecutor:
Given the nature of multiple create feature, the order of execution of code statements is highly important. Consider the example of Book - Publisher which are related through a N:1 relationship.
Here, the insertion of Publisher item has to be completed before the logic for populating the foreign keys of Book item can begin to execute.
To guarantee the correct order of execution as well maintain the transactional aspects of the feature (successfully rollback all the inserted items if there any failures), the following equivalent synchronous methods were needed.
ExecuteQuery
ExecuteQueryAgainstDb
SetManagedIdentityAccessTokenIfAny
Read
ExtractResultSetFromDbDataReader
GetResultProperties
Note: These synchronous methods are used only for the create part of the multiple create feature. The selection set resolution still continues to use the asynchronous equivalent of these methods and takes advantage of the benefits they offer. All the other features continue to use the asynchronous version of these methods.
6. Selection Set Resolution
Selection set resolution involves fetching the requested fields on items created as result of current mutation operation. The logic for selection set resolution of a point create mutation operation is the same as the one that is present today. However, to resolve the selection set of a many type multiple create operation such as
createbooks
orcreateBookmarks_multiple
, new logic had to be introduced.The current limitation with
SqlInsertQueryStructure
is that it can accept PKs of 0 or 1 item. It cannot accept PKs for a list of items which is precisely what is needed for many type multiple create mutation operation.New constructor for
SqlInsertQueryStructure
to accept PKs for a list of items is introduced.To account for a list of PKs in the eventual
SELECT
statement, the logic for adding predicates is updated.Logic changes in
SqlQueryStructure.AddPrimaryKeyPredicates()
andMsSqlQueryBuilder.BuildQueryForMultipleCreateOperation()
Note: Exact database queries for each scenario and query plan analysis are added in the design doc.
For all the entities involved in the selection set,
read
policy will be honored.7. Some more notes about the feature
When the feature flag for multiple create is disabled (or) when used with database types that do not support multiple create operations, a create mutation will follow the current logic of processing a create mutation. Only for databases that support multiple create and when the feature is enabled, this new logic kicks in.
Rationale: Accounting for related entity fields in every step of processing the mutation request is uncessary. So, when it can be determined that the intention is to not use multiple create feature (either feature flag is disabled or database type does not support the feature), a lot of unnecessary logic can be totally skipped.
All or None Behavior: The scope of the transaction applies to the mutation operation as a whole. Irrespective of a point or many type multiple create operation, either all of the items will be created successfully or none of them will be created.
8. Introduced new entities and tables for testing
The intent of introducing these new tables and entities is to define relationships between only through config file and validate different create mutation scenarios. There are no foreign key constraints defined in any of the tables. The relationships defined are only through config file.
9. Database Queries executed:
Consider the following multiple create mutation request
The following database queries are executed in the same order:
CREATE STATEMENTS
1. Publisher
2. Book
3. Website User
4. Reviews
5. Authors
6. Linking table
SELECT STATEMENT
How was this tested?
Sample Request(s)