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.

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

issue/2953 #2962

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

issue/2953 #2962

wants to merge 3 commits into from

Conversation

mkoertgen
Copy link

fixes #2953

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 18, 2024
@mkoertgen mkoertgen mentioned this pull request Oct 18, 2024
Open
@meistermeier
Copy link
Collaborator

Thanks a lot. I hope to have a look later this week (🤞 )

@meistermeier meistermeier self-assigned this Oct 28, 2024
@meistermeier meistermeier removed the status: waiting-for-triage An issue we've not yet triaged label Oct 30, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 30, 2024
meistermeier
meistermeier reviewed Oct 30, 2024
View reviewed changes
src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java Outdated Show resolved Hide resolved
src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java
@@ -744,21 +748,24 @@ public void deleteAll(Class<?> domainType) {
});
}

private <T> ExecutableQuery<T> createExecutableQuery(Class<T> domainType, Statement statement) {
return createExecutableQuery(domainType, null, statement, Collections.emptyMap());
private <T> ExecutableQuery<T> createExecutableQuery(Class<T> domainType, Statement statement, boolean readOnly) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we place name it like readOnlyTransaction or similar? Of course it makes sense in the current context of the transaction PR, but I know that in a few weeks, I need to think a second more about what this parameter stands for.

@meistermeier
Copy link
Collaborator

Thanks again for this PR. I think it makes sense in this way, even though I am not completely happy with the syntax of readOnlyTxManager....(...., readOnly=true). It feels somehow like a duplication. On the other hand I see no way how the write-mode transaction manager in the executable query should know that it is about to join an ongoing transaction.

Something that is missing in the PR is the support for the reactive template manager (you're missing all the fun ;) )
Unfortunately there is no validate method on the reactive transaction manager that will help us (you) to verify that this is working, or I cannot find it right now.

@mkoertgen
Copy link
Author

Hi @meistermeier ,

Will add the reactive part. Thanks for reviewing 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meistermeier meistermeier meistermeier left review comments

Assignees

@meistermeier meistermeier

Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Neo4jTemplate.DefaultExecutableQuery.getSingleResult() always executes in non-readonly transaction
3 participants
@mkoertgen @meistermeier @spring-projects-issues