Fix problem running MERGE inside FOREACH in compatibility mode #7146
Conversation
| @@ -36,71 +36,70 @@ | |||
| { | |||
| public Lock exclusiveLock( Supplier<Statement> stmtSupplier, PropertyContainer container ) | |||
There was a problem hiding this comment.
This method is also used by the TopLevelTransaction or PlaceboTransaction in order to implement Transaction.acquireWriteLock(PropertyContainer).
The Supplier<Statement> passed in through that API will actually "acquire a new" statement (the implementation of course will acquire a new statement by incrementing the reference count on the current statement, and to make things more confusing this is done via a method call called currentStatement despite it actually providing a new statement). Thus not closing the statement in this method will cause unbalanced statements for transactions that do explicit locking.
systay
force-pushed
the
foreach-merge-2.3
branch
3 times, most recently
from
c588bb4
to
e406cc6
Compare
|
Thanks @thobe. WDYT about this solution? |
| */ | ||
| public Lock exclusiveLock( Statement statement, PropertyContainer container ) | ||
| { | ||
| if ( container instanceof Node ) |
There was a problem hiding this comment.
Now shared and exclusive have different behaviour when they deal with statement: not closing and closing it, which is bad
There was a problem hiding this comment.
Not really - there are is a exclusive/share version of the methods that take a Supplier<Statement>, but only a single exclusive version that does not close the statement after itself.
I agree that it's not pretty - this is just a bug fix in the 2.3 maintenance branch. When merging into the 3.0 branch, we should just do a null-merge, because this has already been fixed "for reals" there
There was a problem hiding this comment.
Perhaps we should rename the second kind of exclusiveLock something like exclusiveLockForCypherRuntime
There was a problem hiding this comment.
BTW @systay you are claiming that the fix is needed in 2.3, but the PR is issued against 3.0 and it is also labelled as such, mistake?
|
@thobe since you have merged this PR now, does it need to be backported to 2.3? we merge it in 3.0 and the name of the branch we merged from contains "2.3" |
|
@davidegrohmann no, the problem is only in the 2.3 compatibility mode in 3.x (at least so I'm told) |
No description provided.