Skip to content

Cypher25 default language #2372

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

Merged
merged 65 commits into from
Jul 1, 2025

Conversation

renetapopova
Copy link
Collaborator

Replaces #2153

Copy link
Contributor

@Hunterness Hunterness left a comment

Choose a reason for hiding this comment

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

Only got started on the review, have looked at 11/21 files and have all the big ones left. But figured I'd post the comments I have so far as I'm pausing for the day.

Copy link
Contributor

@Hunterness Hunterness left a comment

Choose a reason for hiding this comment

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

Still have 2 files left to look at, the privilege file (how did you get over 700 lines changed from our 90 something on the previous PR? XD) and the manage aliases for standard database file (only like 200 more lines changed than the previous PR :P)

Copy link
Contributor

@Hunterness Hunterness left a comment

Choose a reason for hiding this comment

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

Review of alias page

Copy link
Contributor

@Hunterness Hunterness left a comment

Choose a reason for hiding this comment

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

Started on the privilege file updates

Copy link
Contributor

@Hunterness Hunterness left a comment

Choose a reason for hiding this comment

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

Next chunk of the privilege file updates

Copy link
Contributor

@Hunterness Hunterness left a comment

Choose a reason for hiding this comment

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

Got through the file, now everything have been looked at once

@renetapopova
Copy link
Collaborator Author

renetapopova commented Jun 24, 2025

@Hunterness, could you please check where it should be SET DEFAULT LANGUAGE CYPHER {5|25}, [SET DEFAULT LANGUAGE CYPHER {5|25}], [DEFAULT LANGUAGE CYPHER {5|25}], DEFAULT LANGUAGE CYPHER {5|25}? I think they are inconsistent across Cypher 5 and Cypher 25, as well as across commands.

@mnd999
Copy link
Contributor

mnd999 commented Jun 24, 2025

@renetapopova just checking, you do know that SET DEFAULT LANGUAGE is also in Cypher 5?

@renetapopova
Copy link
Collaborator Author

@renetapopova just checking, you do know that SET DEFAULT LANGUAGE is also in Cypher 5?

Is this a question to my question above or separate?

@renetapopova
Copy link
Collaborator Author

In some places, it is with SET, in some without. Sometimes, it's optional, and sometimes, it's not. Hence, my question above.

@mnd999
Copy link
Contributor

mnd999 commented Jun 24, 2025

@renetapopova separate. I haven't been able to find the Cypher 5 docs for the release, and tracking the PR history is quite tricky. I want to make sure we documented SET DEFAULT LANGUAGE in both language versions.

From looking at the parser, SET is mandatory on ALTER DATABASE, and from Cypher 25 optional on CREATE DATABASE

@renetapopova renetapopova requested a review from Hunterness June 27, 2025 15:58
Copy link
Contributor

@Hunterness Hunterness left a comment

Choose a reason for hiding this comment

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

There are still some open comments from before as well

Comment on lines 12 to 13
This chapter describes how to manage local and remote standard databases, composite databases, and database aliases. +
All databases are managed using the Cypher administration commands.
Copy link
Contributor

Choose a reason for hiding this comment

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

first thing is addressed, second isn't

====

You can use the `IMPERSONATE` privilege to allow a user to impersonate another user.
Copy link
Contributor

Choose a reason for hiding this comment

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

we have it above all other tables like this, so feels more consistent to keep it above (it's not just this page, also for the database and alias pages as well as the index/constraint/show/terminate pages in the cypher manual)

@renetapopova
Copy link
Collaborator Author

I think there are two comments left.
Screenshot 2025-06-30 at 15 52 08
I don't know what you're referring to, and I can't see the comment in the file.

Screenshot 2025-06-30 at 15 52 55 The note is above the table, as it is on all other pages. Where do you mean it should go?

@renetapopova renetapopova requested a review from Hunterness July 1, 2025 07:54
Copy link
Contributor

@Hunterness Hunterness left a comment

Choose a reason for hiding this comment

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

These are the remaining comments I have and some new ones from the latest updates

====

You can use the `IMPERSONATE` privilege to allow a user to impersonate another user.
Copy link
Contributor

Choose a reason for hiding this comment

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

But it is above the table, as on all pages. What do you mean?

Currently nothing to do on this: It was just in response to your previous comment in the comment thread around talking about moving it below the table and me disagreeing to that and wanting it to stay above

@renetapopova renetapopova requested a review from Hunterness July 1, 2025 11:52
Copy link
Contributor

@Hunterness Hunterness left a comment

Choose a reason for hiding this comment

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

Just a small error left that sneaked in

@renetapopova renetapopova merged commit bbf6bb7 into neo4j:cypher-25 Jul 1, 2025
4 checks passed
@renetapopova renetapopova deleted the cypher25-default-language branch July 1, 2025 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants