-
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
Added the nested, updating, and chained subqueries CIP #100
base: master
Are you sure you want to change the base?
Conversation
I'm not informed enough to offer meaningful feedback here, but as a user of Neo4j, this looks great! Thank you for putting this together. =) |
The only thing that made me go "hmm, interesting" was that subqueries' variables (variable bindings) persist into the outer query. How does that work if you have multiple independent subqueries that happen to (coincidentally) share a variable name? (E.g. something generic like It would feel a bit more intuitive to me if only the But again, I'm not informed enough to know if this feedback is reasonable or flawed. |
@aseemk Based on the examples, I'm guessing that it is only the |
Even more so, the intent is to not allow any shadowing, i.e. you cannot rebind variable from the outer query in the |
+1 |
I like the simplicity of this proposal, but I wonder how well it scales to other things we would/might want to do with/as subqueries in the future. I was actually hoping we would be able to replace union with subqueries, rather than just allowing post-union processing by using union in a subquery, because I find union-queries hard to read with the constituent parts not being strongly delineated. I've also held some hope that we would be able to use sub-queries as parameters to procedures, and there might be other use cases we would want to use sub-queries for that I can't think of right now. |
I have added the 'NOT READY FOR REVIEW' label only because we still need to consider path bindings for subqueries. We'll add this in a fortnight or so, when @boggle and I are back from leave. In the meantime, much content regarding the new |
Hi @zazi Thanks for your comment. I was wondering if you could provide an example (or link) in which correlated subqueries are supported in SPARQL? According to the W3C link in the CIP, it appears to be the case that 'vanilla' SPARQL cannot support correlated subqueries, but it may very well be the case that a vendor-specific implementation does. If this is the case, I would love to know, and, of course, this shall be added to the CIP. |
@petraselmer I'm not 100% an expert in this field. Nevertheless, (from my understanding) I'm supporting the answer to this question http://answers.semanticweb.com/questions/24508/does-sparql-11-support-correlated-subqueries ;) |
|
||
Owing to the bottom-up nature of SPARQL query evaluation, the subqueries are evaluated logically first, and the results are projected up to the outer query. | ||
|
||
Only variables projected out of the subquery will be visible, or in scope, to the outer query. |
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 would tend to say that this is not a disadvantage, but rather then a (good) feature.
For readability issues, I would recommend to add all queries in plain, natural language sentences as well. |
MERGE (c:Child {id: x}) | ||
MERGE (r)-[:PARENT]->(c) | ||
} | ||
---- |
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.
Might be nice to explain the difference between the preceding example and what it would be without the DO
and just having the MERGE
calls following UNWIND
. I assume it's simply keeping the same cardinality (i.e. the DO
itself has a cardinality of 1) and not introducing any new identifiers, but it might be helpful to be explicit about that and why it's helpful.
Furthermore, some example data that illustrate the queries might be helpful (i.e. a small data set and the result sets of the different queries). |
This would be solved by the addition of TCK scenarios for this feature (which is planned). Do you agree? |
@thobe passing subqueries as arguments to procedures is out of scope for this CIP. @thobe How do you envision replacing |
+1, I'm waiting for this |
In which Neo4j release is/will this change be included ? |
@ric81 That is currently not known. Do note that this repository manages the specification and standardisation of Cypher as a language, which is separate from Neo4j the graph database (although this decoupling is fairly new and not yet fully mature). |
|
||
The inner mandatory query is any inner match query. | ||
|
||
Moreover, any inner match query from which the trailing final `RETURN` clause has been omitted may also be used as an inner mandatory query. |
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 think this needs examples to demonstrate the use of these. As well as the difference between having a RETURN
clause and leaving it out.
In my understanding MANDATORY
without RETURN
would be like EXISTS
, but as a clause with the capability of failing the query rather than a boolean expression. Interestingly that use of MANDATORY
would not affect the cardinality of the query, whereas MANDATORY
with RETURN
could potentially increase the cardinality of the query (although never decrease it, since that would mean that no matches were found).
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.
We're actually thinking about making RETURN
mandatory on MANDATORY
match based on the reason that this will be the 95% use case and having to name in the remaining 5% isn't a big deal.
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.
The benefit would be less special casing. Then again, maybe someone might find it very natural to leave off RETURN
.
This CIP doesn't contain any formal syntax description in EBNF format. |
- Clear separation between additive and replacing semantics - Additive semantics for nesting with {} - Replacing semantics for flat composition - Use THEN for discard cardinality - Use WITH|RETURN|YIELD NOTHING for discard fields
746478d
to
061b2fd
Compare
061b2fd
to
3ed1ca9
Compare
This is part of the redesign of Cypher for adding support for working with multiple graphs that targets Cypher 10.
Rendered:
https://github.com/petraselmer/openCypher/blob/CIP-nested-subqueries/cip/1.accepted/CIP2016-06-22-nested-updating-and-chained-subqueries.adoc