-
Notifications
You must be signed in to change notification settings - Fork 63
Not a path pattern expression #1431
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
base: dev
Are you sure you want to change the base?
Conversation
acdd42a to
8b4efc1
Compare
Lojjs
left a comment
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.
Really nice new content, just some minor feedback
| 2+d|Rows: 5 | ||
| |=== | ||
|
|
||
| When '<exp>', then the label expression predicate results in `null`, e.g. if `p` is `null`, then `p:!CEO` results in `null`. |
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.
It feels like something is missing here, it should not say just When '<exp>' right?
modules/ROOT/pages/expressions/predicates/label-expression-predicates.adoc
Show resolved
Hide resolved
| 2+d|Rows: 5 | ||
| |=== | ||
|
|
||
| If `r` is `null`, then the label expression predicate, e.g. `r:WORKS_FOR|REPORTS_TO` results in `null`. |
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.
Perhaps you should add the sentence about using coalesce to change nulls to default values here as well. Having read the whole page it is clear, but if someone is looking at this example only it might be a bit confusing that this sentence mentions null and the result below does not
modules/ROOT/pages/expressions/predicates/path-pattern-expressions.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/expressions/predicates/path-pattern-expressions.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/expressions/predicates/path-pattern-expressions.adoc
Outdated
Show resolved
Hide resolved
| [source, error] | ||
| ---- | ||
| Invalid input 'Node' for `p`. Expected to be Boolean. | ||
| ---- |
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.
Not necessary for this PR, but we should maybe think about starting to use the GQL errors in the Cypher manual instead.
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.
You mean the showing the code? — because the message is what I get on 2025.10.1, i.e. it is the message that comes with the code.
I guess that is a topic for the error team sync. It should have PM involvement, considering how error should be shown in docs (e.g. with position and carrot) and be done as a concerted change to the whole docs.
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.
Yes, showing the code and status description rather than or additional to the message. I will write it up for an upcoming error meeting, I agree it does not make much sense to do it for one random example but it should be consistently applied
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.
Added it like this to next meeting:
[LB] Displaying errors in Cypher docs
Currently errors in the Cypher manual looks like this with the current message:
<screen shot from this PR>Should we instead/additionally show GQLSTATUS and status description (with a link to the error manual maybe)? How do we keep status descriptions up to date in that case?
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.
Linking to the status code doc would be very reasonable. The docs colleagues maybe can even come up if a macro so that in the ascii doc sources you only have put the marco with the status code and it will generate a nice up-to-date presentation of the error incl. link to the respective status code doc page.
Co-authored-by: Louise Berglund <[email protected]>
|
@Lojjs I have addressed all your comments. |
Lojjs
left a comment
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.
Good work!
asked Lidia Zuin to create an image based on our image creation figma files (you can always reach out to the docs team for such images btw) |
rsill-neo4j
left a comment
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.
left a few editorial suggestions - make sure they still reflect the intended meaning :)
modules/ROOT/pages/expressions/predicates/label-expression-predicates.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/expressions/predicates/label-expression-predicates.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/expressions/predicates/label-expression-predicates.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/expressions/predicates/label-expression-predicates.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/expressions/predicates/label-expression-predicates.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/expressions/predicates/label-expression-predicates.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/expressions/predicates/path-pattern-expressions.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/expressions/predicates/path-pattern-expressions.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/expressions/predicates/path-pattern-expressions.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/expressions/predicates/path-pattern-expressions.adoc
Outdated
Show resolved
Hide resolved
Co-authored-by: Richard Sill <[email protected]>
|
@rsill-neo4j I happily applied all your suggestions. Lidia's picture is lovely. Please sort the cheat sheet tags as you see fit. I kept them basic, but I do not really have a clue. Otherwise, I guess you take it from here!? Thanks. |
|
@hvub is this general information, or a new feature in the sense that we should add it to the additions page? (also, does it apply to all releases?) |
|
@rsill-neo4j the label expression predicate is in Cypher as long as I am at Neo4j. It has been slightly modified since when GQL's label expression where added, which was like 3 years ago or so. So, I'd it a general addition to the docs, something that is simply missing. |
|
This PR includes documentation updates New pages: Updated pages: |
decided against adding this to the cheat sheet - it's ok to have this only as something to discover when reading the docs pages |
This adds some documentation about potential confusion of some parenthesized expression with path pattern expressions.
While doing so, I could not find documentation of the label expression predicate, so I add some too.
The new SVG is a manually edited copy of another. I do not have an SVG editor handy, so I was a bit limited. If you have better tools feel free to adjust, of course.
Fixes SURF-165