When using multi-version concurrency, re-running a prepared statement with the…

When using multi-version concurrency, re-running a prepared statement with the same parameters would sometimes give the same result even if another connection committed a change (the previous result was sometimes re-used incorrectly).

h2/src/docsrc/html/changelog.html

@@ -18,7 +18,13 @@ Change Log
 <h1>Change Log</h1>
 
 <h2>Next Version (unreleased)</h2>
-<ul><li>Concurrently preparing a statement and altering a table could throw a table not found exception.
+<ul><li>When using multi-version concurrency, re-running a prepared statement with the same parameters 
+    would sometimes give the same result even if another connection committed a change (the previous result was sometimes 
+    re-used incorrectly).
+</li><li>When using SELECT_FOR_UPDATE_MVCC (lock only the selected rows when using MVCC),
+    the selected rows were sometimes not locked correctly.
+</li><li>When using Lucene 3, the index files were not always closed when the database was closed.
+</li><li>Concurrently preparing a statement and altering a table could throw a table not found exception.
 <li><li>When concurrently preparing many statements with a subquery, in some cases the query didn't run
     (Column "..." must be in the GROUP BY list).
 </li><li>CallableStatement: now the syntax "{? = CALL...}" is supported as well.

h2/src/main/org/h2/engine/UndoLogRecord.java

@@ -6,10 +6,8 @@
  */
 package org.h2.engine;
 
-import java.util.ArrayList;
 import org.h2.constant.ErrorCode;
 import org.h2.constant.SysProperties;
-import org.h2.index.Index;
 import org.h2.message.DbException;
 import org.h2.result.Row;
 import org.h2.store.Data;
@@ -255,11 +253,7 @@ public class UndoLogRecord {
      * It commits the change to the indexes.
      */
     public void commit() {
-        ArrayList<Index> indexes = table.getIndexes();
-        for (int i = 0, size = indexes.size(); i < size; i++) {
-            Index index = indexes.get(i);
-            index.commit(operation, row);
-        }
+        table.commit(operation, row);
     }
 
     /**

h2/src/main/org/h2/table/RegularTable.java

@@ -157,6 +157,14 @@ public class RegularTable extends TableBase {
         analyzeIfRequired(session);
     }
 
+    public void commit(short operation, Row row) {
+        lastModificationId = database.getNextModificationDataId();
+        for (int i = 0, size = indexes.size(); i < size; i++) {
+            Index index = indexes.get(i);
+            index.commit(operation, row);
+        }
+    }
+
     private void checkRowCount(Session session, Index index, int offset) {
         if (SysProperties.CHECK && !database.isMultiVersion()) {
             if (!(index instanceof PageDelegateIndex)) {

h2/src/main/org/h2/table/Table.java

@@ -187,6 +187,16 @@ public abstract class Table extends SchemaObjectBase {
      */
     public abstract void addRow(Session session, Row row);
 
+    /**
+     * Commit an operation (when using multi-version concurrency).
+     *
+     * @param operation the operation
+     * @param row the row
+     */
+    public void commit(short operation, Row row) {
+        // nothing to do
+    }
+
     /**
      * Check if this table supports ALTER TABLE.
      *

h2/src/test/org/h2/test/db/TestTransaction.java

@@ -15,6 +15,7 @@ import java.sql.Savepoint;
 import java.sql.Statement;
 import java.util.ArrayList;
 import java.util.Random;
+import org.h2.engine.Constants;
 import org.h2.test.TestBase;
 import org.h2.util.New;
 
@@ -91,17 +92,21 @@ public class TestTransaction extends TestBase {
         Connection conn = getConnection("transaction");
         conn.setAutoCommit(false);
         Statement stat = conn.createStatement();
-        stat.execute("create table test(id int primary key) as select 1");
+        stat.execute("create table test(id int primary key, name varchar)");
+        stat.execute("insert into test values(1, 'Hello'), (2, 'World')");
         conn.commit();
-        PreparedStatement prep = conn.prepareStatement("select * from test for update");
+        PreparedStatement prep = conn.prepareStatement("select * from test where id = 1 for update");
         prep.execute();
         // releases the lock
         conn.commit();
         prep.execute();
         Connection conn2 = getConnection("transaction");
         conn2.setAutoCommit(false);
+        if (config.mvcc && Constants.VERSION_MINOR >= 3) {
+            conn2.createStatement().execute("update test set name = 'Welt' where id = 2");
+        }
         try {
-            conn2.createStatement().execute("select * from test");
+            conn2.createStatement().execute("update test set name = 'Hallo' where id = 1");
             fail();
         } catch (SQLException e) {
             assertKnownException(e);

h2/src/test/org/h2/test/mvcc/TestMvcc1.java

@@ -7,6 +7,7 @@
 package org.h2.test.mvcc;
 
 import java.sql.Connection;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
@@ -61,6 +62,8 @@ public class TestMvcc1 extends TestBase {
         if (!config.mvcc) {
             return;
         }
+        ResultSet rs;
+
         // TODO Prio 1: document: exclusive table lock still used when altering
         //     tables, adding indexes, select ... for update; table level locks are
         //     checked
@@ -148,9 +151,9 @@ public class TestMvcc1 extends TestBase {
         s1.execute("create table test(id int primary key, name varchar(255))");
         s1.execute("insert into test values(1, 'y')");
         c1.commit();
-        s2.execute("select * from test for update");
+        s2.execute("select * from test where id = 1 for update");
         try {
-            s1.execute("insert into test values(2, 'x')");
+            s1.execute("delete from test");
             fail();
         } catch (SQLException e) {
             // lock timeout expected
@@ -195,10 +198,17 @@ public class TestMvcc1 extends TestBase {
         c1.commit();
         assertResult("1", s2, "SELECT COUNT(*) FROM TEST");
         s1.executeUpdate("DELETE FROM TEST");
+        PreparedStatement p2 = c2.prepareStatement("select count(*) from test");
+        rs = p2.executeQuery();
+        rs.next();
+        assertEquals(1, rs.getInt(1));
         assertResult("1", s2, "SELECT COUNT(*) FROM TEST");
         assertResult("0", s1, "SELECT COUNT(*) FROM TEST");
         c1.commit();
         assertResult("0", s2, "SELECT COUNT(*) FROM TEST");
+        rs = p2.executeQuery();
+        rs.next();
+        assertEquals(0, rs.getInt(1));
         c1.commit();
         c2.commit();
         s1.execute("DROP TABLE TEST");
@@ -378,7 +388,7 @@ public class TestMvcc1 extends TestBase {
         } catch (SQLException e) {
             assertKnownException(e);
         }
-        ResultSet rs = s1.executeQuery("select * from test order by id");
+        rs = s1.executeQuery("select * from test order by id");
         assertTrue(rs.next());
         assertEquals(1, rs.getInt(1));
         assertEquals("Hello", rs.getString(2));