Repository: clickhouse/clickhouse-java
Issue: Incorrect update count (0.3.2-patch9)

Hi! 

In `com.clickhouse.jdbc.internal.ClickHouseStatementImpl#updateResult`:

```
...
// FIXME apparently this is not always true
if (currentUpdateCount <= 0L) {
    currentUpdateCount = 1L;
}
...
```

Yes, this isn’t always true :)

**Examples:**

```
Statement statement = connection.createStatement();
statement.execute(sql);
int updateCount = statement.getUpdateCount();
```

1. `String sql = “CREATE TABLE new_table(x INT) engine = Memory;”`
Expected `updateCount`: 0
Actual `updateCount`: 1

2. `String sql = "INSERT INTO new_table VALUES (1), (2), (3);"`
Expected `updateCount`: 3
Actual `updateCount`: 1

Changed files:
clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseResponseSummary.java
clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java
clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java
clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHouseStatementTest.java

diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseResponseSummary.java b/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseResponseSummary.java
index c75c9b8a..33bc639b 100644
--- a/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseResponseSummary.java
+++ b/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseResponseSummary.java
@@ -10,6 +10,8 @@ import java.util.concurrent.atomic.AtomicReference;
 public class ClickHouseResponseSummary implements Serializable {
     private static final long serialVersionUID = 6241261266635143197L;
 
+    static final String ERROR_CANNOT_UPDATE = "Sealed summary cannot be updated";
+
     public static final ClickHouseResponseSummary EMPTY = new ClickHouseResponseSummary(null, null, true);
 
     /**
@@ -18,6 +20,8 @@ public class ClickHouseResponseSummary implements Serializable {
     public static final class Progress implements Serializable {
         private static final long serialVersionUID = -1447066780591278108L;
 
+        static final Progress EMPTY = new Progress(0L, 0L, 0L, 0L, 0L);
+
         private final long read_rows;
         private final long read_bytes;
         private final long total_rows_to_read;
@@ -61,6 +65,11 @@ public class ClickHouseResponseSummary implements Serializable {
         public long getWrittenBytes() {
             return written_bytes;
         }
+
+        public boolean isEmpty() {
+            return read_rows == 0L && read_bytes == 0L && total_rows_to_read == 0L && written_rows == 0L
+                    && written_bytes == 0L;
+        }
     }
 
     /**
@@ -69,6 +78,8 @@ public class ClickHouseResponseSummary implements Serializable {
     public static class Statistics implements Serializable {
         private static final long serialVersionUID = -7744796632866829161L;
 
+        static final Statistics EMPTY = new Statistics(0L, 0L, 0L, false, 0L);
+
         private final long rows;
         private final long blocks;
         private final long allocated_bytes;
@@ -112,6 +123,10 @@ public class ClickHouseResponseSummary implements Serializable {
         public long getRowsBeforeLimit() {
             return rows_before_limit;
         }
+
+        public boolean isEmpty() {
+            return rows == 0L && blocks == 0L && allocated_bytes == 0L && !applied_limit && rows_before_limit == 0L;
+        }
     }
 
     private final AtomicReference<Progress> progress;
@@ -139,9 +154,15 @@ public class ClickHouseResponseSummary implements Serializable {
      * @param sealed   whether the summary is sealed
      */
     protected ClickHouseResponseSummary(Progress progress, Statistics stats, boolean sealed) {
-        this.progress = new AtomicReference<>(progress != null ? progress : new Progress(0L, 0L, 0L, 0L, 0L));
-        this.stats = new AtomicReference<>(stats != null ? stats : new Statistics(0L, 0L, 0L, false, 0L));
-        this.updates = new AtomicInteger(1);
+        if (progress == null) {
+            progress = Progress.EMPTY;
+        }
+        if (stats == null) {
+            stats = Statistics.EMPTY;
+        }
+        this.progress = new AtomicReference<>(progress);
+        this.stats = new AtomicReference<>(stats);
+        this.updates = new AtomicInteger(progress.isEmpty() && stats.isEmpty() ? 0 : 1);
 
         this.sealed = sealed;
     }
@@ -159,6 +180,10 @@ public class ClickHouseResponseSummary implements Serializable {
      * @return increased update counter
      */
     public int update() {
+        if (sealed) {
+            throw new IllegalStateException(ERROR_CANNOT_UPDATE);
+        }
+
         return this.updates.incrementAndGet();
     }
 
@@ -169,7 +194,7 @@ public class ClickHouseResponseSummary implements Serializable {
      */
     public void update(Progress progress) {
         if (sealed) {
-            throw new IllegalStateException("Sealed summary cannot be updated");
+            throw new IllegalStateException(ERROR_CANNOT_UPDATE);
         }
 
         if (progress != null) {
@@ -179,7 +204,7 @@ public class ClickHouseResponseSummary implements Serializable {
 
     public void update(Statistics stats) {
         if (sealed) {
-            throw new IllegalStateException("Sealed summary cannot be updated");
+            throw new IllegalStateException(ERROR_CANNOT_UPDATE);
         }
 
         if (stats != null) {
@@ -228,4 +253,8 @@ public class ClickHouseResponseSummary implements Serializable {
     public int getUpdateCount() {
         return updates.get();
     }
+
+    public boolean isEmpty() {
+        return progress.get().isEmpty() && stats.get().isEmpty();
+    }
 }
diff --git a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java
index 82e20603..d1b47522 100644
--- a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java
+++ b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java
@@ -95,8 +95,13 @@ public class ClickHouseStatementImpl extends JdbcWrapper
             } catch (Exception e) {
                 throw SqlExceptionUtils.handle(e);
             } finally {
-                if (i + 1 < len && response != null) {
+                if (response == null) {
+                    // something went wrong
+                } else if (i + 1 < len) {
                     response.close();
+                    response = null;
+                } else {
+                    updateResult(stmt, response);
                 }
             }
         }
@@ -166,18 +171,17 @@ public class ClickHouseStatementImpl extends JdbcWrapper
 
     protected int executeInsert(String sql, InputStream input) throws SQLException {
         boolean autoTx = connection.getAutoCommit() && connection.isTransactionSupported();
-        ClickHouseResponseSummary summary = null;
         Mutation req = request.write().query(sql, queryId = connection.newQueryId()).data(input);
         try (ClickHouseResponse resp = autoTx
                 ? req.executeWithinTransaction(connection.isImplicitTransactionSupported())
                 : req.transaction(connection.getTransaction()).sendAndWait();
                 ResultSet rs = updateResult(new ClickHouseSqlStatement(sql, StatementType.INSERT), resp)) {
-            summary = resp.getSummary();
+            // ignore
         } catch (Exception e) {
             throw SqlExceptionUtils.handle(e);
         }
 
-        return summary != null && summary.getWrittenRows() > 0L ? (int) summary.getWrittenRows() : 1;
+        return (int) currentUpdateCount;
     }
 
     protected ClickHouseSqlStatement getLastStatement() {
@@ -212,23 +216,17 @@ public class ClickHouseStatementImpl extends JdbcWrapper
     }
 
     protected ResultSet updateResult(ClickHouseSqlStatement stmt, ClickHouseResponse response) throws SQLException {
-        ResultSet rs = null;
         if (stmt.isQuery() || !response.getColumns().isEmpty()) {
             currentUpdateCount = -1L;
             currentResult = new ClickHouseResultSet(stmt.getDatabaseOrDefault(getConnection().getCurrentDatabase()),
                     stmt.getTable(), this, response);
-            rs = currentResult;
         } else {
-            currentUpdateCount = response.getSummary().getWrittenRows();
-            // FIXME apparently this is not always true
-            if (currentUpdateCount <= 0L) {
-                currentUpdateCount = 1L;
-            }
-            currentResult = null;
             response.close();
+            currentUpdateCount = stmt.isDDL() ? 0L
+                    : (response.getSummary().isEmpty() ? 1L : response.getSummary().getWrittenRows());
+            currentResult = null;
         }
-
-        return rs == null ? newEmptyResultSet() : rs;
+        return currentResult;
     }
 
     protected ClickHouseStatementImpl(ClickHouseConnectionImpl connection, ClickHouseRequest<?> request,
@@ -303,18 +301,8 @@ public class ClickHouseStatementImpl extends JdbcWrapper
         }
 
         parseSqlStatements(sql);
-
-        ClickHouseResponse response = getLastResponse(null, null, null);
-
-        try {
-            return updateResult(getLastStatement(), response);
-        } catch (Exception e) {
-            if (response != null) {
-                response.close();
-            }
-
-            throw SqlExceptionUtils.handle(e);
-        }
+        getLastResponse(null, null, null);
+        return currentResult != null ? currentResult : newEmptyResultSet();
     }
 
     @Override
@@ -326,14 +314,11 @@ public class ClickHouseStatementImpl extends JdbcWrapper
 
         parseSqlStatements(sql);
 
-        ClickHouseResponseSummary summary = null;
         try (ClickHouseResponse response = getLastResponse(null, null, null)) {
-            summary = response.getSummary();
+            return currentUpdateCount;
         } catch (Exception e) {
             throw SqlExceptionUtils.handle(e);
         }
-
-        return summary != null ? summary.getWrittenRows() : 1L;
     }
 
     @Override
diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java
index c3ec0334..f8b54c32 100644
--- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java
+++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java
@@ -845,6 +845,7 @@ public class ClickHousePreparedStatementTest extends JdbcIntegrationTest {
     @Test(dataProvider = "statementAndParams", groups = "integration")
     public void testExecuteWithOrWithoutParameters(String tableSuffix, String query, Class<?> clazz,
             boolean hasResultSet, String[] params, boolean checkTable) throws SQLException {
+        int expectedRowCount = "ddl".equals(tableSuffix) ? 0 : 1;
         String tableName = "test_execute_ps_" + tableSuffix;
         query = query.replace("$table", tableName);
         Properties props = new Properties();
@@ -932,7 +933,7 @@ public class ClickHousePreparedStatementTest extends JdbcIntegrationTest {
                 if (hasResultSet) {
                     Assert.assertThrows(SQLException.class, () -> ps.executeLargeBatch());
                 } else {
-                    Assert.assertEquals(ps.executeLargeBatch(), new long[] { 1L });
+                    Assert.assertEquals(ps.executeLargeBatch(), new long[] { expectedRowCount });
                 }
                 if (checkTable)
                     checkTable(stmt, "select * from " + tableName, params);
@@ -950,7 +951,7 @@ public class ClickHousePreparedStatementTest extends JdbcIntegrationTest {
                 if (hasResultSet) {
                     Assert.assertThrows(SQLException.class, () -> ps.executeBatch());
                 } else {
-                    Assert.assertEquals(ps.executeBatch(), new int[] { 1 });
+                    Assert.assertEquals(ps.executeBatch(), new int[] { expectedRowCount });
                 }
                 if (checkTable)
                     checkTable(stmt, "select * from " + tableName, params);
@@ -973,7 +974,7 @@ public class ClickHousePreparedStatementTest extends JdbcIntegrationTest {
             if (hasResultSet) {
                 Assert.assertEquals(ps.executeLargeBatch(), new long[] { Statement.EXECUTE_FAILED });
             } else {
-                Assert.assertEquals(ps.executeLargeBatch(), new long[] { 1L });
+                Assert.assertEquals(ps.executeLargeBatch(), new long[] { expectedRowCount });
             }
             if (checkTable)
                 checkTable(stmt, "select * from " + tableName, params);
@@ -988,7 +989,7 @@ public class ClickHousePreparedStatementTest extends JdbcIntegrationTest {
             if (hasResultSet) {
                 Assert.assertEquals(ps.executeBatch(), new int[] { Statement.EXECUTE_FAILED });
             } else {
-                Assert.assertEquals(ps.executeBatch(), new int[] { 1 });
+                Assert.assertEquals(ps.executeBatch(), new int[] { expectedRowCount });
             }
             if (checkTable)
                 checkTable(stmt, "select * from " + tableName, params);
diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHouseStatementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHouseStatementTest.java
index 4861c29e..681bdeca 100644
--- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHouseStatementTest.java
+++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHouseStatementTest.java
@@ -173,7 +173,7 @@ public class ClickHouseStatementTest extends JdbcIntegrationTest {
             // [update] tbl a [set] a.b = 1 where a.b != 1[ settings mutation_async=0]
             // alter table tbl a update a.b = 1 where a.b != 1
             conn.setClientInfo("ApplicationName", "333");
-            Assert.assertEquals(conn.createStatement().executeUpdate("update test_mutation set b = 22 where b = 1"), 0);
+            Assert.assertEquals(conn.createStatement().executeUpdate("update test_mutation set b = 22 where b = 1"), 1);
 
             Assert.assertThrows(SQLException.class,
                     () -> stmt.executeUpdate("update non_existing_table set value=1 where key=1"));
@@ -407,7 +407,7 @@ public class ClickHouseStatementTest extends JdbcIntegrationTest {
             rs = stmt.executeQuery("drop table if exists non_existing_table");
             Assert.assertNotNull(rs, "Should never be null");
             Assert.assertNull(stmt.getResultSet(), "Should be null");
-            Assert.assertEquals(stmt.getUpdateCount(), 1);
+            Assert.assertEquals(stmt.getUpdateCount(), 0);
             Assert.assertFalse(rs.next(), "Should has no row");
         }
     }
@@ -519,7 +519,7 @@ public class ClickHouseStatementTest extends JdbcIntegrationTest {
             stmt.addBatch("drop table if exists non_existing_table2");
             stmt.addBatch("drop table if exists non_existing_table3");
             int[] results = stmt.executeBatch();
-            Assert.assertEquals(results, new int[] { 1, 1, 1 });
+            Assert.assertEquals(results, new int[] { 0, 0, 0 });
         }
     }