This repository has been archived by the owner on Sep 3, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 147
Primary keys #227
Open
calendardays
wants to merge
18
commits into
neo4j-graphql:master
Choose a base branch
from
calendardays:primaryKeys
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Primary keys #227
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ullable field = primary key, except for _id. Matching is based on composite of all primary keys. Types with no primary keys cannot be matched, and match-requiring mutations are not generated for them. This update affects generation and translation of Input types, Update/Delete mutations, and Add/Remove mutations.
…t match all nodes of that type. Relation mutations that touch these nodes will be created, but 'from' and 'to' arguments/fields will not exist for node types with no primary keys---instead, all nodes of those types will be matched and relationships will be Added/Removed for all of them.
…move, 'data' is now nullable if the corresponding relation type has no primary keys.
…, with all fields for FullInput and only primary key fields for PKInput
…ir and primary key properties of edge, then updates non-PK properties of edge
…requiring PK-only input type for data arg
…. AddInput uses nullability from schema, ChangeInput and RemoveInput make non-nullable fields == primary key fields, RemoveInput drops non-PK fields.
…f the relation has one or more data fields but no primary keys
… non-nullable if they are present (because the related nodes should exist), and all relation properties have the same nullability in the return payload as in the schema relation type definition
…e now return lists
Forgot one other collateral thing in here: In the current |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goals
Allow for each node type and each relation type to have any number (>=0) of primary keys (PKs) specified in the GraphQL schema, and use the collection of a type's PKs for matching when resolving mutations.
The problems this branch solves
My initial motivation involved a database with
Person
nodes who had both afirstName
and alastName
. When I try to use the auto-generatedUpdatePerson
mutation to, say, change John Doe's phone number, in the currentneo4j-graphql-js
master branch, the result is that every John gets his phone number changed. And this behavior is not obvious, becauseUpdatePerson
returns a single record (Person
) rather than a list of records ([Person]
), so only information about the John with the lowest_id
is returned to the client.Additionally, I noticed that relation type mutations don't have all the benefits of node type mutations.
Add
andRemove
correspond toCreate
andDelete
, but there is no correlate forUpdate
. AndRemove
works in a way that assumes you never create multiple relations with the same label between the same two nodes. If you do,Remove
as it currently exists will delete all of them without any ability to specify.How this branch solves those problems
In this new branch, every field marked non-nullable (
!
) in the schema will be treated as a PK, except for_id
,from
, andto
. When resolving mutations, the collection of all PKs is used as a composite key for matching a node or relation.Ideally there should be a distinct way to indicate PKs, such as a
@pk
directive or some other clever solution, so that PKs and non-nullable fields can be distinct sets when desired. But I didn't want to push for any one particular solution at this stage, and the current implementation is sufficient for demonstrating the benefits of fleshed-out PKs.Node mutations
Update
andDelete
mutations treat all PKs (of the node type) as required arguments and use all of them for matching.Update
are used for updating.Update
andDelete
for that type will match and affect ALL nodes with the matching label.Node input types
from
andto
arguments of relation mutations, require all PKs of the corresponding node types as arguments.from
argument in a relation mutation, instead that mutation does not accept afrom
argument, and the mutation resolves by matching all relations that are "from" any node of the corresponding label (provided those relations match all constraints from other mutation args). The same goes forto
.Relation mutations
Change
which is the relation version ofUpdate
.Change
andRemove
mutations treat all PKs (of the relation type) as required arguments and use all of them for matching.Remove
mutations will require adata
arg if the relation type has one or more PKs.Change
are used for updating.Change
andRemove
for that type will match and affect ALL relations with the matching label that connect the two matched nodes.Returning lists
Update
,Delete
,Change
, orRemove
in cases where a type has no PKs, so that all nodes or relations with a given label will be matched.Update
mutation for each node type instead of having a whole bunch ofUpdate
mutations depending on which properties you feel like matching and which ones you feel like updating at the moment, which is the flexibility you get by interacting directly with a Neo4j database and writing custom cypher queries for each thing you want to do.Nullability
If this work is extended, it could become possible to specify PKs in a different way, i.e., not by just treating all non-nullable (
!
) fields as PKs. (It would be as simple as redefining the functiongetPrimaryKeys
inutils.js
.) When deciding whether the fields and args of auto-generated types and mutations should be nullable, sometimes it makes sense to follow schema-defined nullability, and other times it makes sense to follow schema-defined PKness.I use schema-defined nullability to set the nullability of:
Create
andAdd
argsI use PKness to set the nullability of:
Update
,Delete
,Change
, andRemove
argsIf a node type has no PKs, then the corresponding input type is not allowed as a
from
orto
argument to mutations.If a relation type has no PKs, then the corresponding input type is not allowed as a
data
argument toRemove
mutations, is optional as adata
arg forAdd
mutations, and is still required as adata
arg forChange
mutations (because you have to change something!).Collateral damage
This isn't relevant to PK features, but I also refactored the
src/*.js
files slightly in my first commit in order to get rid of circular import dependencies that were causing problems for me.Notes
I am not an expert at this. This is my first PR. These features are useful to me, but if you are not interested in taking the project in this direction, that's cool. If you like my ideas but my implementation is incomplete or broken, I would not be shocked.
Would love to discuss further, answer questions about design choices, or take your advice and make revisions.
Thanks!