-
Notifications
You must be signed in to change notification settings - Fork 150
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
Specifying Cypher constraints #166
base: master
Are you sure you want to change the base?
Conversation
6a73206
to
73f9909
Compare
73f9909
to
1fd0cbf
Compare
1fd0cbf
to
942570a
Compare
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.
Looks good! Some comments...
I thought the idea was that we were going to specify a syntax for constraints without specifying which particular constraints an implementation should support. The syntax definition here explicitly talks only about |
CIP has now been reworked to try and fit the model discussed. |
* `UNIQUE` - Ensures that each row for a column must have a unique value. | ||
* `PRIMARY KEY` - A combination of a `NOT NULL` and `UNIQUE`. Ensures that a column (or a combination of two or more columns) has a unique identity, reducing the resources required to locate a specific record in a table. | ||
* `FOREIGN KEY` - Ensures the referential integrity of the data in one table matches values in another table. | ||
* `CHECK` - Ensures that the value in a column meets a specific condition |
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.
Maybe we should have a leading keyword like that for non-UNIQUE constraints as well?
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.
if so, I would propose the word THAT
, since it reads nicely...
CREATE CONSTRAINT FOR (x:Foo) REQUIRE THAT ...
4121971
to
22a7f30
Compare
On the high level there are two reasons for this:
Diving further into these reasons, starting with the word "primary":
As for the aspects of primary keys in relational databases that do not apply to the property graph model:
The only reason I can think of for introducing the concept of a primary key in Cypher is for being able to map Cypher onto a relational database model. If that is the case I would much rather see this proposed from a vendor working on such a mapping, since they would have the insight into what needs to be modeled. I do think that the notion of a unique indexed key of mandatory properties is helpful, and I see the benefit of elevating such a concept to the status of receiving its own syntax, but I don't think |
==== Mutability | ||
|
||
Once a constraint has been created, it may not be amended. | ||
Should a user wish to change its definition, it has to be dropped and recreated with an updated structure. |
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.
Should we have a note here that transactional implementations could do both the dropping and recreation in the same transaction so that the constraint is atomically mutated? This would of course allow leaving the old constraint in place should the creation of the new constraint fail.
I wonder if the notion of unique key (that at the moment are erroneously called primary key) should really be a constraint, or if it should have its own syntax, something like:
The reason for this being that it actually implies multiple constraints, and typically also an index. Since it is a composed concept like that, perhaps it would be sensible to elevate it to being a syntactical concept of its own. In the syntax for this, if accepted, we should allow an optional name for the key as well, just like we do for constraints. |
The CIP now uses |
60c584f
to
4ef7b32
Compare
- Rename `constrait-expr` to `constraint-predicate` - Limit scope of `UNIQUE` to single properties only - Update examples to reflect `PRIMARY KEY`
- Remove erroneous example for composing `NODE KEY` with `UNIQUE` and `exists()` - Rephrase example section to describe `NODE KEY` more accurately.
- Add missing case for when an error should be raised
Add test for DROP
For improved source readability
- use same EBNF style as @hvub did in opencypher#493 - use links to connect to grammar constructs in openCypher grammar spec - modify constraint operators to be suffix-modeled, ala IS NOT NULL - introduce 'grouped-expression' concept
Extend definition to reference grouped expression
- Update definition to reference grouped expression - Reformulate equivalence example to use IS NOT NULL and IS UNIQUE over a grouped expression
Move ahead of referencing sections to ease readability of CIP
- Including links to these from the CIP - Also update examples for the parser tests
This CIP has now been updated. See commit history for a summary of new models. TCK tests cannot be added as the CIP mandates no particular concrete constraint predicates to be enforced by every implementation of Cypher. However, I will exemplify TCK scenarios that one could consider if one were to implement the NODE KEY constraint:
|
Specifies syntax for constraints and specifies three concrete constraints: node property uniqueness, node property existence, and relationship property existence.
CIP