Skip to content
This repository has been archived by the owner on Sep 3, 2021. It is now read-only.

Primary keys #227

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Conversation

calendardays
Copy link

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 a firstName and a lastName. When I try to use the auto-generated UpdatePerson mutation to, say, change John Doe's phone number, in the current neo4j-graphql-js master branch, the result is that every John gets his phone number changed. And this behavior is not obvious, because UpdatePerson 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 and Remove correspond to Create and Delete, but there is no correlate for Update. And Remove 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, and to. 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

  • Auto-generated node Update and Delete mutations treat all PKs (of the node type) as required arguments and use all of them for matching.
  • All non-PK args passed to Update are used for updating.
  • If a node type has no PKs, Update and Delete for that type will match and affect ALL nodes with the matching label.

Node input types

  • Node input types, which are supplied as the from and to arguments of relation mutations, require all PKs of the corresponding node types as arguments.
  • If a node type has no PKs, then wherever it would be a from argument in a relation mutation, instead that mutation does not accept a from 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 for to.

Relation mutations

  • There is a new auto-generated relation mutation called Change which is the relation version of Update.
  • Auto-generated relation Change and Remove mutations treat all PKs (of the relation type) as required arguments and use all of them for matching.
  • This means that Remove mutations will require a data arg if the relation type has one or more PKs.
  • All non-PK args passed to Change are used for updating.
  • If a relation type has no PKs, Change and Remove for that type will match and affect ALL relations with the matching label that connect the two matched nodes.

Returning lists

  • Mutations that might affect multiple nodes or multiple relations at once return lists. This means Update, Delete, Change, or Remove in cases where a type has no PKs, so that all nodes or relations with a given label will be matched.
  • Note that returning single objects elsewhere encodes an assumption that your database must be maintained in such a way that any unique set of values for a node's or relation's PKs must match at most one unique node or relation. This is the normal interpretation of "primary keys," but may not be obvious, because "primary keys" are not a native concept to graph databases. Instead, they are used here as a trade-off so that, e.g., we can generate one Update mutation for each node type instead of having a whole bunch of Update 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 function getPrimaryKeys in utils.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 and Add args

I use PKness to set the nullability of:

  • Update, Delete, Change, and Remove args
  • Node input type fields

If a node type has no PKs, then the corresponding input type is not allowed as a from or to argument to mutations.

If a relation type has no PKs, then the corresponding input type is not allowed as a data argument to Remove mutations, is optional as a data arg for Add mutations, and is still required as a data arg for Change 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!

colton added 18 commits April 1, 2019 15:58
…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
…. 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
@calendardays
Copy link
Author

Forgot one other collateral thing in here:

In the current master branch, if you run an auto-generated Remove mutation and it matches two nodes in your database but those nodes don't actually have any relations between them in your database, the mutation still returns information about those nodes instead of returning null. That didn't seem right to me: I felt that no-ops should return null to the client, even in a case like this. So one of my changes modified that behavior so that Remove (and Change) only return data on stuff that actually got mutated. If the database was not changed, they return null.

@michaeldgraham
Copy link
Collaborator

#608

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants