Merge pull request xerial#72 from batterseapower/master

Test and fix bug in execute(String sql)

src/main/java/org/sqlite/core/CoreStatement.java

@@ -69,12 +69,15 @@ protected boolean exec() throws SQLException {
         if (rs.isOpen())
             throw new SQLException("SQLite JDBC internal error: rs.isOpen() on exec.");
 
+        boolean success = false;
         boolean rc = false;
         try {
             rc = db.execute(this, null);
+            success = true;
         }
         finally {
             resultsWaiting = rc;
+            if (!success) db.finalize(this);
         }
 
         return db.column_count(pointer) != 0;
@@ -94,11 +97,14 @@ protected boolean exec(String sql) throws SQLException {
             throw new SQLException("SQLite JDBC internal error: rs.isOpen() on exec.");
 
         boolean rc = false;
+        boolean success = false;
         try {
             rc = db.execute(sql);
+            success = true;
         }
         finally {
             resultsWaiting = rc;
+            if (!success) db.finalize(this);
         }
 
         return db.column_count(pointer) != 0;

src/main/java/org/sqlite/core/DB.java

@@ -844,10 +844,13 @@ final synchronized boolean execute(String sql) throws SQLException {
      * @throws SQLException
      */
     public final synchronized int executeUpdate(CoreStatement stmt, Object[] vals) throws SQLException {
-        if (execute(stmt, vals)) {
-            throw new SQLException("query returns results");
+        try {
+            if (execute(stmt, vals)) {
+                throw new SQLException("query returns results");
+            }
+        } finally {
+            reset(stmt.pointer);
         }
-        reset(stmt.pointer);
         return changes();
     }
 

src/main/java/org/sqlite/jdbc3/JDBC3PreparedStatement.java

@@ -48,8 +48,14 @@ public boolean execute() throws SQLException {
         db.reset(pointer);
         checkParameters();
 
-        resultsWaiting = db.execute(this, batch);
-        return columnCount != 0;
+        boolean success = false;
+        try {
+            resultsWaiting = db.execute(this, batch);
+            success = true;
+            return columnCount != 0;
+        } finally {
+            if (!success) db.reset(pointer);
+        }
     }
 
     /**
@@ -66,7 +72,13 @@ public ResultSet executeQuery() throws SQLException {
         db.reset(pointer);
         checkParameters();
 
-        resultsWaiting = db.execute(this, batch);
+        boolean success = false;
+        try {
+            resultsWaiting = db.execute(this, batch);
+            success = true;
+        } finally {
+            if (!success) db.reset(pointer);
+        }
         return getResultSet();
     }
 

src/test/java/org/sqlite/TransactionTest.java

@@ -10,8 +10,7 @@
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
-import java.util.Map;
-import java.util.Properties;
+import java.util.*;
 
 import org.junit.After;
 import org.junit.Before;
@@ -70,6 +69,71 @@ public void close() throws Exception {
         conn3.close();
     }
 
+    private void failedUpdatedPreventedFutureRollback(boolean prepared) throws SQLException {
+        stat1.execute("create table test (c1);");
+        stat1.execute("insert into test values (1);");
+
+        // First transaction starts
+        conn1.setAutoCommit(false);
+        stat1.execute("insert into test values (2);");
+
+        final PreparedStatement pstat2 = prepared ? conn2.prepareStatement("insert into test values (3);") : null;
+
+        // Second transaction starts and tries to complete but fails because first is still running
+        boolean gotException = false;
+        try {
+            conn2.setAutoCommit(false);
+            if (pstat2 != null) {
+                // The prepared case would fail regardless of whether this was "execute" or "executeUpdate"
+                pstat2.execute();
+            } else {
+                // If you changed this to "executeUpdate" instead of "execute", the test would pass
+                stat2.execute("insert into test values (3);");
+            }
+        } catch (SQLException e) {
+            if (e.getMessage().contains("is locked")) {
+                gotException = true;
+            } else {
+                throw e;
+            }
+        }
+        assertTrue(gotException);
+        conn2.rollback();
+        // The test would fail here: the trivial "transaction" created in between the rollback we just
+        // did and this point would fail to commit because "SQL statements in progress"
+        conn2.setAutoCommit(true);
+
+        // First transaction completes
+        conn1.setAutoCommit(true);
+
+        // Second transaction retries
+        conn2.setAutoCommit(false);
+        if (pstat2 != null) {
+            pstat2.execute();
+        } else {
+            stat2.execute("insert into test values (3);");
+        };
+        conn2.setAutoCommit(true);
+
+        final ResultSet rs = stat1.executeQuery("select c1 from test");
+        final Set<Integer> seen = new HashSet<Integer>();
+        while (rs.next()) {
+            assertTrue(seen.add(rs.getInt(1)));
+        }
+
+        assertEquals(new HashSet<Integer>(Arrays.asList(1, 2, 3)), seen);
+    }
+
+    @Test
+    public void failedUpdatePreventedFutureRollbackUnprepared() throws SQLException {
+        failedUpdatedPreventedFutureRollback(false);
+    }
+
+    @Test
+    public void failedUpdatePreventedFutureRollbackPrepared() throws SQLException {
+        failedUpdatedPreventedFutureRollback(true);
+    }
+
     @Test
     public void multiConn() throws SQLException {
         stat1.executeUpdate("create table test (c1);");