Skip to content

add note about database name in query log #1980

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 26, 2024
Merged

add note about database name in query log #1980

merged 2 commits into from
Nov 26, 2024

Conversation

@LinneaAndersson
Copy link
Contributor

In order to know which database the query is targeting, we do some parsing to find if there is a USE-clause (e.g. USE db RETURN 1 AS result). If the parsing fails, we are not able to figure out what database the query should be run at and therefor the field in the log will be .

Please let me know if I can improve the documentation with more details, I just don't want to put information to confuse the user even more.

@LinneaAndersson LinneaAndersson added documentation Improvements or additions to documentation dev team-cypher-composite labels Nov 22, 2024
@renetapopova renetapopova self-requested a review November 22, 2024 10:12
@renetapopova
Copy link
Collaborator

It looks good, but don't we throw an error as well?

@LinneaAndersson
Copy link
Contributor Author

@renetapopova Yes we do, but there has been confusing about why the database field in the log has been <none>. So I just added a tiny comment to the docs :)

renetapopova
renetapopova approved these changes Nov 25, 2024
View reviewed changes
Copy link
Collaborator

@renetapopova renetapopova left a comment

Choose a reason for hiding this comment

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

A tiny editorial suggestion. Otherwise, looks good.

modules/ROOT/pages/monitoring/logging.adoc Outdated

| database
| The database name on which the query is run.
| The database name on which the query is run. This field will be `<none>` if it is not possible to parse and route the query to the correct database.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| The database name on which the query is run. This field will be `<none>` if it is not possible to parse and route the query to the correct database.
| The database name on which the query is run. This field will be `<none>` if it is impossible to parse and route the query to the correct database.

Copy link

Choose a reason for hiding this comment

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

Another editorial suggestion - "route the query to a database" instead of "route a query to the correct database" - I would argue that there might be other reasons a query might not be routed to the user's definition of "correct"!

Copy link
Contributor Author

@LinneaAndersson LinneaAndersson Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

For some reason it felt wrong to say impossibe (don't ask me why 😄). Would it be okay with the change I made to:

"This field will be <none> if the query cannot be parsed and routed to a database."

@renetapopova
Copy link
Collaborator

@LinneaAndersson, in which version do you want this to be published?

@neo-technology-commit-status-publisher
Copy link
Collaborator

neo-technology-commit-status-publisher commented Nov 25, 2024 •
edited
Loading

Thanks for the documentation updates.

The preview documentation has now been torn down - reopening this PR will republish it.

@LinneaAndersson
Copy link
Contributor Author

LinneaAndersson commented Nov 25, 2024 •
edited
Loading

@renetapopova This should be updated for Neo4j 5 (and also in neo4j 25) :) It's nothing new in 5 LTS, it's been there for a long time

@renetapopova renetapopova added cherry-pick-this-to-5.x and removed documentation Improvements or additions to documentation dev labels Nov 26, 2024
@renetapopova renetapopova merged commit da576bf into neo4j:dev Nov 26, 2024
8 checks passed
renetapopova pushed a commit to renetapopova/docs-operations that referenced this pull request Nov 26, 2024
@LinneaAndersson @renetapopova
add note about database name in query log (neo4j#1980)
87f09e4 
In order to know which database the query is targeting, we do some
parsing to find if there is a USE-clause (e.g. `USE db RETURN 1 AS
result`). If the parsing fails, we are not able to figure out what
database the query should be run at and therefor the field in the log
will be <none>.

Please let me know if I can improve the documentation with more details,
I just don't want to put information to confuse the user even more.
@renetapopova renetapopova mentioned this pull request Nov 26, 2024
Merged
renetapopova added a commit that referenced this pull request Nov 26, 2024
@renetapopova @LinneaAndersson
add note about database name in query log (#1980) (#1988)
eb4c3a1 
Cherry-picked from #1980

Co-authored-by: LinneaAndersson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@renetapopova renetapopova renetapopova approved these changes

@htsedebenham htsedebenham htsedebenham approved these changes

Assignees

@htsedebenham htsedebenham

Labels

cherry-pick-this-to-5.x team-cypher-composite

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@LinneaAndersson @renetapopova @neo-technology-commit-status-publisher @htsedebenham