-
Notifications
You must be signed in to change notification settings - Fork 156
Update existential subquery CIP #493
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
Update existential subquery CIP #493
Conversation
4dc23f7 to
cfca711
Compare
97fabf4 to
4e87bde
Compare
…d case sensitivity
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.
A very comprehensive rework, impressive!
I found a few typos that we should fix before merge. Also one comment on TCK correctness.
There are also some suggestions to improve understandability, which are optional.
Thanks a lot for this!
Co-authored-by: Mats Rydberg <[email protected]>
accept suggestion from review Co-authored-by: Mats Rydberg <[email protected]>
Co-authored-by: Mats Rydberg <[email protected]>
Co-authored-by: Mats Rydberg <[email protected]>
Co-authored-by: Mats Rydberg <[email protected]>
Co-authored-by: Mats Rydberg <[email protected]>
Co-authored-by: Mats Rydberg <[email protected]>
Co-authored-by: Mats Rydberg <[email protected]>
Co-authored-by: Mats Rydberg <[email protected]>
Co-authored-by: Mats Rydberg <[email protected]>
Co-authored-by: Mats Rydberg <[email protected]>
Co-authored-by: Mats Rydberg <[email protected]>
|
@hvub Great improvements! Thanks a lot for addressing my comments. |
|
|
||
| Both are allowed as https://raw.githack.com/openCypher/openCypher/master/tools/grammar-production-links/grammarLink.html?p=Atom[<Atom>]s of expressions. | ||
|
|
||
| Note that a <RelationshipsPattern> appearing in an expression is evaluated to a list of paths containing the matches of the pattern. |
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.
Is <RelationshipsPattern> supposed to be a link like below? (Twice in this paragraph)
| Further, https://raw.githack.com/openCypher/openCypher/master/tools/grammar-production-links/grammarLink.html?p=PatternComprehension[<PatternComprehension>] offer a more powerful and syntactically better separated means to get lists of matches for a pattern within expressions. | ||
|
|
||
| The understanding of subqueries in Cypher has evolved and Cypher gained a proper https://raw.githack.com/openCypher/openCypher/master/tools/grammar-production-links/grammarLink.html?p=NullOperatorExpression[<NullOperatorExpression>] (`IS [ NOT ] NULL`), cf. https://github.com/opencypher/openCypher/blob/master/cip/1.accepted/CIP2018-10-29-EXISTS-and-IS-NOT-NULL.adoc[CIP2018-10-29 EXISTS and IS NOT NULL]. | ||
| Both developments and the issues around <RelationshipsPattern>s suggest the need for a refinement of the language constructs that allow testing of existence. |
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.
Is <RelationshipsPattern> supposed to be a link?
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.
I have added the links.
Co-authored-by: Satia Herfert <[email protected]>
Co-authored-by: Satia Herfert <[email protected]>
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.
Looking great!
- 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
This PR updates CIP2015-05-13 EXISTS (rendered view and in conjunction the grammar as well as TCK scenarios.
More specifically, the PR
ExistentialSubqueryExistentialSubqueryas alternative toAtomCaseAlternativestoCaseAlternative, since is represent only a single alternativeWherein the productionPatternComprehensionexists(<X>)with<X> IS NOT NULLon remaining occurrences in TCK scenarios<X> IS [NOT] NULLin Null1 and Null2... IN keys(...)to Graph8 and Map3