-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
In BatchExecutor, close each statement right after it is executed. #1110
Conversation
@@ -149,9 +150,6 @@ public int doUpdate(MappedStatement ms, Object parameterObject) throws SQLExcept | |||
} | |||
return results; | |||
} finally { | |||
for (Statement stmt : statementList) { |
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.
In this changes, If an exception occurred during statement execution, I think there is a case that statement does not close.
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.
Thank you for review - you are right. I will improve the code.
@vlastimil-dolejs Thank you for the PR! @kazuki43zoo Any idea for testing this? I couldn't think of a good one. |
Hi @harawata, it's difficult .. :( |
May possible if use the mock mechanism like Mockito... |
@harawata @vlastimil-dolejs I have one question. |
(Probably) Calling the close method at twice is no problem because the JDBC 4.2 specification("13.1.4 Closing Statement Objects") says as follows:
|
OK. Let's just leave it for now.
+1 |
@kazuki43zoo
So it's safe to call it twice. I think that code that ensures the |
In BatchExecutor, close each statement right after it is executed. Although proper flushing should be sufficient in most cases, this could lower the possibility of an error like "ORA-01000: maximum open cursors exceeded".
…sed as suggested by Kazuki.
fix for #1109