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

Keyword priority #1130

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Keyword priority #1130

wants to merge 5 commits into from

Conversation

hvub
Copy link
Contributor

@hvub hvub commented Dec 9, 2024

Docs update regarding https://github.com/neo-technology/neo4j/pull/28493 (plus a little clarification w.r.t. variable scope in subqueries).

@hvub hvub force-pushed the keyword-priority branch 2 times, most recently from 86b1699 to df08c0c Compare December 9, 2024 16:26
Copy link
Contributor

@loveleif loveleif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

modules/ROOT/pages/syntax/keywords.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/syntax/keywords.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/syntax/keywords.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/syntax/keywords.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/syntax/keywords.adoc Outdated Show resolved Hide resolved
* Labels
* Relationship types

If a snippet of Cypher can validly be a keyword as well as an unquoted identifier, it is always interpreted as a keyword.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering about "snippet" - the term suggests that it can be more than one word, while "keyword" suggests that it's a single one (or rather a string which doesn't contain spaces, thinking of "ALL_SHORTEST_PATHS" etc)

would "expression" work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"expression" has a precise meaning. a + b is an expression to, while e.g. a label identifier alone is not an expression. We do not have a proper term, so I picked something non-technical.

However, I am not married to "snippet". Not really a fan of my choice here in fact.

  • A piece of Cypher
  • A part of Cypher
  • Cypher text
  • ...

any would work.

Or a totally difference angle may be this:

If an unquoted identifier equal to a keyword is user in Cypher in a place where can also the keyword, then it is interpreted as the keyword.

BTW: Feel free to edit the PR directly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, yes, "expression" wouldn't work, you're right :/

what about "piece of Cypher syntax"?


Keywords are words that have a special meaning in Cypher.

The keywords are not recommended to be used as identifiers in the following contexts:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the recommendation strong enough here? if it doesn't work at all, we could say that here and then offer the use of quotes below as a way of still using the keyword as an identifier, if the user wants to do that (if so, remove "unquoted" from line 14)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you mean strong enough? I am not sure I follow.

After all it is a recommendation. User do not have to do that but it will make their live a little less complicated. The only true safe thing are quote identifiers. But Valerio seems not to be in favour of recommending that. Hence, I only hint at it and leave to the reader make this last step of logical inference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the contexts pointed out, it only works when using quotations because otherwise they would be interpreted as the keywords, correct?

in a sense, it's not a recommendation then, rather it just doesn't work without quotations

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. In the specific contexts of ambiguity, which are more subtle then what is listed here, you have to use quotation to get identifier interpretation.

However, the recommendation is deliberately more coarse grained, recommend to us quotes on identifiers (that are labels, relationship types, and variable) equal to keywords in all places.

For instance, the commendation is to not do something like this:

WITH 1 AS RETURN
RETURN RETURN

although is works perfectly fine.

As the example hopefully illustrates, the recommendation is more coarse grained than functionally needed, because

  1. it can be confusing for a query reader if you use keywords as identifiers without quotes even if there is no ambiguity for the system
  2. you do not have to worry about in where and what these exact contexts of actual ambiguity are, i.e. the recommendation is much simpler and more stable of time

BTW:
I removed functions names from the recommendation, but maybe we should add that back in, because the following is likewise confusing:

WITH WITH("foo") AS RETURN
CALL CALL("bar") YIELD YIELD
RETURN RETURN, YIELD

My thinking was that most users do not assign function names. Only if you do have custom functions (UDF) you assigned function names. Nevertheless. I will add function name and procedure them back in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also included parameters, originally. These can never be ambiguous because they are prefixed with a $.

So do we think that

WITH $WITH AS foo
RETURN $RETURN, $AS, foo

is to be avoided? I am open to opinions here.

Note that these are deliberately arcane examples.

There are many instance that are less arcane and confusing, e.g.:

WITH $USER AS foo
RETURN $TEXT, foo

or (without parameters)

WITH "bob" AS USER, "Hello" AS START
RETURN START + USER AS SHOW

I guess, we do not want to scare user too much about the need of quoting. But that want something to point to if they start to complain about too weird cases — which most users will avoid their own sake.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright, let's leave it as is then :) thanks for taking the time to elaborate

1+d|Rows: 1
|===

If any keyword is quoted in backticks (```), such as `++`true`++`, it is always interpreted as an identifier in the given context.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If any keyword is quoted in backticks (```), such as `++`true`++`, it is always interpreted as an identifier in the given context.
If a keyword is quoted in backticks (```), such as `++`true`++`, it is interpreted as an identifier in the given context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with your suggestion.

However, if we like my proposal above, we may want to do something like following here:

A quoted identifier equal to a keyword is interpreted as an identifier (even if it is equal to a keyword).

modules/ROOT/pages/syntax/keywords.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/syntax/keywords.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/syntax/variables.adoc Outdated Show resolved Hide resolved
@neo-technology-commit-status-publisher
Copy link
Collaborator

This PR includes documentation updates
View the updated docs at https://neo4j-docs-cypher-1130.surge.sh

New pages:

Updated pages:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants