ChinmaySKulkarni commented on a change in pull request #746: PHOENIX-5802: 
Connection leaks in UPSERT SELECT/DELETE paths due to 
MutatingParallelIteratorFactory iterator not being closed
URL: https://github.com/apache/phoenix/pull/746#discussion_r399484082
 
 

 ##########
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertSelectIT.java
 ##########
 @@ -252,158 +291,164 @@ public void testUpsertSelectEmptyPKColumn() throws 
Exception {
         Properties props = new Properties();
         props.setProperty(QueryServices.ENABLE_SERVER_SIDE_UPSERT_MUTATIONS,
             allowServerSideMutations);
-        Connection conn = DriverManager.getConnection(getUrl(), props);
-        conn.setAutoCommit(false);
-        String upsert = "UPSERT INTO " + ptsdbTable + "(\"DATE\", val, host) " 
+
-            "SELECT current_date(), x_integer+2, entity_id FROM " + aTable + " 
WHERE a_integer >= ?";
-        PreparedStatement upsertStmt = conn.prepareStatement(upsert);
-        upsertStmt.setInt(1, 6);
-        int rowsInserted = upsertStmt.executeUpdate();
-        assertEquals(4, rowsInserted);
-        conn.commit();
-        conn.close();
-        
+        String upsert;
+        ResultSet rs;
+        int rowsInserted;
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            conn.setAutoCommit(false);
+            upsert = "UPSERT INTO " + ptsdbTable + "(\"DATE\", val, host) " +
+                    "SELECT current_date(), x_integer+2, entity_id FROM " + 
aTable + " WHERE a_integer >= ?";
+            try (PreparedStatement upsertStmt = conn.prepareStatement(upsert)) 
{
+                upsertStmt.setInt(1, 6);
+                rowsInserted = upsertStmt.executeUpdate();
+                assertEquals(4, rowsInserted);
+            }
+            conn.commit();
+        }
+
         String query = "SELECT inst,host,\"DATE\",val FROM " + ptsdbTable;
-        conn = DriverManager.getConnection(getUrl(), props);
-        PreparedStatement statement = conn.prepareStatement(query);
-        ResultSet rs = statement.executeQuery();
-        
+        try (Connection conn = DriverManager.getConnection(getUrl(), props);
+        PreparedStatement statement = conn.prepareStatement(query)) {
+            rs = statement.executeQuery();
+
+            Date now = new Date(EnvironmentEdgeManager.currentTimeMillis());
+            assertTrue (rs.next());
+            assertEquals(null, rs.getString(1));
+            assertEquals(ROW6, rs.getString(2));
+            assertTrue(rs.getDate(3).before(now) );
+            assertEquals(null, rs.getBigDecimal(4));
+
+            assertTrue (rs.next());
+            assertEquals(null, rs.getString(1));
+            assertEquals(ROW7, rs.getString(2));
+            assertTrue(rs.getDate(3).before(now) );
+            assertTrue(BigDecimal.valueOf(7).compareTo(rs.getBigDecimal(4)) == 
0);
+
+            assertTrue (rs.next());
+            assertEquals(null, rs.getString(1));
+            assertEquals(ROW8, rs.getString(2));
+            assertTrue(rs.getDate(3).before(now) );
+            assertTrue(BigDecimal.valueOf(6).compareTo(rs.getBigDecimal(4)) == 
0);
+
+            assertTrue (rs.next());
+            assertEquals(null, rs.getString(1));
+            assertEquals(ROW9, rs.getString(2));
+            assertTrue(rs.getDate(3).before(now) );
+            assertTrue(BigDecimal.valueOf(5).compareTo(rs.getBigDecimal(4)) == 
0);
+
+            assertFalse(rs.next());
+        }
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            conn.setAutoCommit(true);
+            upsert = "UPSERT INTO " + ptsdbTable + "(\"DATE\", val, inst) " +
+                    "SELECT \"DATE\"+1, val*10, host FROM " + ptsdbTable;
+            try (PreparedStatement upsertStmt = conn.prepareStatement(upsert)) 
{
+                rowsInserted = upsertStmt.executeUpdate();
+                assertEquals(4, rowsInserted);
+            }
+            conn.commit();
+        }
         Date now = new Date(EnvironmentEdgeManager.currentTimeMillis());
-        assertTrue (rs.next());
-        assertEquals(null, rs.getString(1));
-        assertEquals(ROW6, rs.getString(2));
-        assertTrue(rs.getDate(3).before(now) );
-        assertEquals(null, rs.getBigDecimal(4));
-        
-        assertTrue (rs.next());
-        assertEquals(null, rs.getString(1));
-        assertEquals(ROW7, rs.getString(2));
-        assertTrue(rs.getDate(3).before(now) );
-        assertTrue(BigDecimal.valueOf(7).compareTo(rs.getBigDecimal(4)) == 0);
-        
-        assertTrue (rs.next());
-        assertEquals(null, rs.getString(1));
-        assertEquals(ROW8, rs.getString(2));
-        assertTrue(rs.getDate(3).before(now) );
-        assertTrue(BigDecimal.valueOf(6).compareTo(rs.getBigDecimal(4)) == 0);
-        
-        assertTrue (rs.next());
-        assertEquals(null, rs.getString(1));
-        assertEquals(ROW9, rs.getString(2));
-        assertTrue(rs.getDate(3).before(now) );
-        assertTrue(BigDecimal.valueOf(5).compareTo(rs.getBigDecimal(4)) == 0);
-
-        assertFalse(rs.next());
-        conn.close();
-        
-        conn = DriverManager.getConnection(getUrl(), props);
-        conn.setAutoCommit(true);
-        upsert = "UPSERT INTO " + ptsdbTable + "(\"DATE\", val, inst) " +
-            "SELECT \"DATE\"+1, val*10, host FROM " + ptsdbTable;
-        upsertStmt = conn.prepareStatement(upsert);
-        rowsInserted = upsertStmt.executeUpdate();
-        assertEquals(4, rowsInserted);
-        conn.commit();
-        conn.close();
-        
         Date then = new Date(now.getTime() + QueryConstants.MILLIS_IN_DAY);
         query = "SELECT host,inst, \"DATE\",val FROM " + ptsdbTable + " where 
inst is not null";
-        conn = DriverManager.getConnection(getUrl(), props);
-        statement = conn.prepareStatement(query);
-        
-        rs = statement.executeQuery();
-        assertTrue (rs.next());
-        assertEquals(null, rs.getString(1));
-        assertEquals(ROW6, rs.getString(2));
-        assertTrue(rs.getDate(3).after(now) && rs.getDate(3).before(then));
-        assertEquals(null, rs.getBigDecimal(4));
-        
-        assertTrue (rs.next());
-        assertEquals(null, rs.getString(1));
-        assertEquals(ROW7, rs.getString(2));
-        assertTrue(rs.getDate(3).after(now) && rs.getDate(3).before(then));
-        assertTrue(BigDecimal.valueOf(70).compareTo(rs.getBigDecimal(4)) == 0);
-        
-        assertTrue (rs.next());
-        assertEquals(null, rs.getString(1));
-        assertEquals(ROW8, rs.getString(2));
-        assertTrue(rs.getDate(3).after(now) && rs.getDate(3).before(then));
-        assertTrue(BigDecimal.valueOf(60).compareTo(rs.getBigDecimal(4)) == 0);
-        
-        assertTrue (rs.next());
-        assertEquals(null, rs.getString(1));
-        assertEquals(ROW9, rs.getString(2));
-        assertTrue(rs.getDate(3).after(now) && rs.getDate(3).before(then));
-        assertTrue(BigDecimal.valueOf(50).compareTo(rs.getBigDecimal(4)) == 0);
-        
-        assertFalse(rs.next());
-        conn.close();
-        
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props);
 
 Review comment:
   Here too, spaces added because of additional indentation because of 
try-with-resources.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to