Skip to content
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

Add Neo4j index extension CIP #197

Open
wants to merge 30 commits into
base: master
from
Open

Conversation

@Mats-SX
Copy link
Member

@Mats-SX Mats-SX commented Mar 3, 2017

This reflects a Neo4j extension of the constraints syntax suggested in #166 to support index creation/deletion.

Direct link to CIP

@boggle boggle added the settled label Mar 27, 2017
@Mats-SX Mats-SX force-pushed the Mats-SX:neo4j-indexes branch 2 times, most recently from 394c1cb to 7e54204 Jul 26, 2019
Mats-SX and others added 24 commits Dec 14, 2016
- General outline
- Describe the node uniqueness constraint
- Define domain for uniqueness
- Define domain for existence
- Use hex integers for rgb examples
- Specify general constraint language
- Specify `UNIQUE` operator
- Clearly define semantics for domain and expressions
- List all concrete constraints in example section
- Add several more examples
They are more appropriate under the Semantics and Syntax sections, respectively.
Move Errors section
- Remove TODO
- Add example using larger pattern
- Add example using multiple `exists()`
- 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
Mats-SX and others added 6 commits Jul 22, 2019
Add test for DROP
- NODE KEY not PRIMARY KEY
- Reference to constraints syntax
- Properly define domain
- Expanded example to explain domain definition and consequences
- Error cases
- Add names to examples
@Mats-SX Mats-SX force-pushed the Mats-SX:neo4j-indexes branch from 7e54204 to 2cebf70 Jul 26, 2019
@Mats-SX
Copy link
Member Author

@Mats-SX Mats-SX commented Jul 26, 2019

This CIP is now also ready for review. It builds on top of #166 so there's no need to review changes that come from that PR here. The reason to build it on top is because this CIP makes many references to the general Constraint Syntax CIP.

@sherfert
Copy link
Contributor

@sherfert sherfert commented Jul 30, 2019

I have some questions:

  • Having node-pattern in the grammar, would that not allow writing CREATE INDEX foo FOR (a:Foo:Bar) ON a.prop? I.e. I think we cannot allow multiple labels or no labels, or if we do, we should explain that neo4j does not currently support that and what the semantics are.

  • I know that neo4j does not store null-properties in indexes, but is there a fundamental reason why we would want to enforce that? I know we have been discussing null-indexes for index-backed-order-by, so I think it can be a valuable thing that we do not want to forbid.

  • Is there a recommendation on how systems should generate unique names for constraints/indexes? I think that would be valuable.

@sherfert sherfert self-assigned this Jul 30, 2019
@Mats-SX Mats-SX removed the settled label Aug 30, 2019
@Mats-SX
Copy link
Member Author

@Mats-SX Mats-SX commented Aug 30, 2019

CLG review finds on @sherfert's points:

  • We'll go with general syntax for patterns and implementation limitations
  • Explicitly forbidding null values will be removed from this CIP
  • No recommendation is required
@Mats-SX
Copy link
Member Author

@Mats-SX Mats-SX commented Aug 30, 2019

From LangStar review:

  • Extend with OPTIONS syntax to specify particular index details (such as implementation type)
  • Allow not only node patterns, but a general pattern including relationships
  • Require property expressions to be delimited using parentheses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.