xtern commented on code in PR #2906:
URL: https://github.com/apache/ignite-3/pull/2906#discussion_r1430330752


##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcStatement.java:
##########
@@ -472,7 +462,7 @@ public boolean getMoreResults(int curr) throws SQLException 
{
             switch (curr) {
                 case CLOSE_CURRENT_RESULT:
                     if (curRes > 0) {
-                        resSets.get(curRes - 1).close0();
+                        resSets.get(curRes - 1).close0(false);

Review Comment:
   Why we are calling `close0` with `false` flag here?
   
   The following example demonstrate resource leakage (or may be I missed 
something):
   ```
       // test can be added to ItJdbcMultiStatementSelfTest
       @Test
       public void testX() throws Exception {
           long initial = openCursorsRegistered();
           
           stmt.executeUpdate("CREATE TABLE TESTX(ID INT PRIMARY KEY)");
           stmt.getMoreResults();
           stmt.close();
   
           assertEquals(0L, initial - openCursorsRegistered());
       }
   ```
   
   As I see, the current combination in `close0` of `isQuery`, `finished`, etc. 
flags were being used because we were not registering cursors for DML, DDL (and 
possibly other) queries.
   
   Since we are now registering cursors for this types of queries, I think it 
is worth reviewing the current conditions in `close0`, because now they look 
too complex and leading to errors :sunglasses: .



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to