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

Subclauses CIP #200

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

Subclauses CIP #200

wants to merge 6 commits into from

Conversation

@Mats-SX
Copy link
Member

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

This CIP is in response to #194

CIP

@Mats-SX Mats-SX added the CIP label Mar 6, 2017
Fixes #194
@Mats-SX Mats-SX force-pushed the Mats-SX:limit-cip branch from 7f10456 to 2967f79 Mar 6, 2017
Copy link
Contributor

@petraselmer petraselmer left a comment

Nice one! Just a few comments


== Background

This CIP is a proposal in answer to link:https://github.com/opencypher/openCypher/issues/194[CIR-2017-194].

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

"answer" -> "response"

=== Semantics

The `LIMIT` subclause prevents records passing through its parent clause after the specified amount of rows, as determined by the limit expression, has been processed.
For these semantics to be well defined, the limit expression must be constant over the query lifetime, such as parameters or literals.

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

Can this be expanded on a bit more? It's not 100% clear what is being said. If need be, an example could be provided.

This comment has been minimized.

@Mats-SX

Mats-SX Mar 8, 2017
Author Member

Expanded and gave two examples.


==== Updating queries

The use of `LIMIT` opens the possibility for certain performance optimisations.

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

"opens the possibility" -> "opens up the possibility"


=== Semantics

The `LIMIT` subclause prevents records passing through its parent clause after the specified amount of rows, as determined by the limit expression, has been processed.

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

"limit expression" -> hmmm... it reads a bit awkwardly at present. Maybe something such as "expression provided to the LIMIT clause" or something. Maybe even formatting would help, such as all in italics, or LIMIT expression?

This comment has been minimized.

@Mats-SX

Mats-SX Mar 8, 2017
Author Member

How about

the expression used as argument to LIMIT

?

This comment has been minimized.

@petraselmer

petraselmer Mar 8, 2017
Contributor

That works - just include "the argument"

==== Updating queries

The use of `LIMIT` opens the possibility for certain performance optimisations.
Clauses that come early in the query do not have to be evaluated over the full dataset, just enough to reach the subsequent limit.

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

"that come early" -> "appearing earlier"

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

"do not have to be evaluated over the full dataset, just enough to reach the subsequent limit."
->
"only need to be evaluated until the limit is reached, as opposed to evaluating the entire dataset."

Clauses that come early in the query do not have to be evaluated over the full dataset, just enough to reach the subsequent limit.
These optimisations are however not always applicable in combination with updating clauses.
Semantics between clauses is defined such that _all_ of a previous clause is processed (logically) before _any_ of a subsequent clause is processed.
This means that _all_ side effects must happen before a `LIMIT` is allowed to halt the processing of records in preceding clauses.

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

"halt" -> "terminate"

Semantics between clauses is defined such that _all_ of a previous clause is processed (logically) before _any_ of a subsequent clause is processed.
This means that _all_ side effects must happen before a `LIMIT` is allowed to halt the processing of records in preceding clauses.

Consider the below query:

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

"below query"
->
"following query"


Consider the below query:

.Create a producer for each item, return first 100 product ids.

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

Format "producer"

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

"item, return first 100 product ids"
->
"item, returning the first 100 product ids"

This comment has been minimized.

@Mats-SX

Mats-SX Mar 8, 2017
Author Member

I don't think producer needs formatting here. The example talks about items and producers as concepts, and I think it's clear that these are then represented by nodes with matching labels.


If the user intention is to only do a partial update of the graph, the query must be rewritten:

.Create a producer for the 100 first items, return their product ids.

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

"100 first items, return their product ids"
->
"top 100 items, and return their product ids"

This comment has been minimized.

@petraselmer

petraselmer Mar 6, 2017
Contributor

Format "producer"

- Rename file to correct name for the CIP
- Improve language
@Mats-SX Mats-SX force-pushed the Mats-SX:limit-cip branch from 60344fe to 3a3f198 Mar 8, 2017
Copy link
Contributor

@thobe thobe left a comment

This should be extended to also include ORDER BY, and SKIP.

In fact all of the following sub-clauses should always go together:

  • WHERE
  • ORDER BY
  • SKIP
  • LIMIT
@Mats-SX
Copy link
Member Author

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

@thobe Extended!

@Mats-SX Mats-SX changed the title Add the LIMIT CIP Add the Subclauses CIP Mar 31, 2017
@Mats-SX Mats-SX changed the title Add the Subclauses CIP Subclauses CIP May 5, 2017
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

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