Merge pull request #1336 from h2database/minor-refactoring

Minor refactorings

h2/src/main/org/h2/mvstore/db/MVPrimaryIndex.java

@@ -56,11 +56,8 @@ public class MVPrimaryIndex extends BaseIndex {
         mapName = "table." + getId();
         Transaction t = mvTable.getTransactionBegin();
         dataMap = t.openMap(mapName, keyType, valueType);
-        dataMap.map.setVolatile(!indexType.isPersistent());
+        dataMap.map.setVolatile(!table.isPersistData() || !indexType.isPersistent());
         t.commit();
-        if (!table.isPersistData() || !indexType.isPersistent()) {
-            dataMap.map.setVolatile(true);
-        }
         Value k = dataMap.map.lastKey();    // include uncommitted keys as well
         lastKey.set(k == null ? 0 : k.getLong());
     }

h2/src/main/org/h2/mvstore/db/MVSecondaryIndex.java

@@ -66,7 +66,7 @@ public final class MVSecondaryIndex extends BaseIndex implements MVIndex {
         ValueDataType valueType = new ValueDataType();
         Transaction t = mvTable.getTransactionBegin();
         dataMap = t.openMap(mapName, keyType, valueType);
-        dataMap.map.setVolatile(!indexType.isPersistent());
+        dataMap.map.setVolatile(!table.isPersistData() || !indexType.isPersistent());
         t.commit();
         if (!keyType.equals(dataMap.getKeyType())) {
             throw DbException.throwInternalError(

h2/src/main/org/h2/mvstore/db/MVSpatialIndex.java

@@ -101,7 +101,7 @@ public class MVSpatialIndex extends BaseIndex implements SpatialIndex, MVIndex {
         spatialMap = db.getMvStore().getStore().openMap(mapName, mapBuilder);
         Transaction t = mvTable.getTransactionBegin();
         dataMap = t.openMap(spatialMap);
-        dataMap.map.setVolatile(!indexType.isPersistent());
+        dataMap.map.setVolatile(!table.isPersistData() || !indexType.isPersistent());
         t.commit();
     }
 
@@ -201,7 +201,7 @@ public class MVSpatialIndex extends BaseIndex implements SpatialIndex, MVIndex {
         Iterator<SpatialKey> cursor = spatialMap.keyIterator(null);
         TransactionMap<SpatialKey, Value> map = getMap(session);
         Iterator<SpatialKey> it = map.wrapIterator(cursor, false);
-        return new MVStoreCursor(session, it);
+        return new MVStoreCursor(session, it, mvTable);
     }
 
     @Override
@@ -215,7 +215,7 @@ public class MVSpatialIndex extends BaseIndex implements SpatialIndex, MVIndex {
                 spatialMap.findIntersectingKeys(getKey(intersection));
         TransactionMap<SpatialKey, Value> map = getMap(session);
         Iterator<SpatialKey> it = map.wrapIterator(cursor, false);
-        return new MVStoreCursor(session, it);
+        return new MVStoreCursor(session, it, mvTable);
     }
 
     private SpatialKey getKey(SearchRow row) {
@@ -230,18 +230,6 @@ public class MVSpatialIndex extends BaseIndex implements SpatialIndex, MVIndex {
                 (float) env.getMinY(), (float) env.getMaxY());
     }
 
-    /**
-     * Get the row with the given index key.
-     *
-     * @param key the index key
-     * @return the row
-     */
-    SearchRow getRow(SpatialKey key) {
-        SearchRow searchRow = mvTable.getTemplateRow();
-        searchRow.setKey(key.getId());
-        return searchRow;
-    }
-
     @Override
     public MVTable getTable() {
         return mvTable;
@@ -324,7 +312,7 @@ public class MVSpatialIndex extends BaseIndex implements SpatialIndex, MVIndex {
      * @param session the session
      * @return the map
      */
-    TransactionMap<SpatialKey, Value> getMap(Session session) {
+    private TransactionMap<SpatialKey, Value> getMap(Session session) {
         if (session == null) {
             return dataMap;
         }
@@ -335,17 +323,19 @@ public class MVSpatialIndex extends BaseIndex implements SpatialIndex, MVIndex {
     /**
      * A cursor.
      */
-    class MVStoreCursor implements Cursor {
+    private static class MVStoreCursor implements Cursor {
 
         private final Session session;
         private final Iterator<SpatialKey> it;
+        private final MVTable mvTable;
         private SpatialKey current;
         private SearchRow searchRow;
         private Row row;
 
-        public MVStoreCursor(Session session, Iterator<SpatialKey> it) {
+        public MVStoreCursor(Session session, Iterator<SpatialKey> it, MVTable mvTable) {
             this.session = session;
             this.it = it;
+            this.mvTable = mvTable;
         }
 
         @Override
@@ -363,7 +353,8 @@ public class MVSpatialIndex extends BaseIndex implements SpatialIndex, MVIndex {
         public SearchRow getSearchRow() {
             if (searchRow == null) {
                 if (current != null) {
-                    searchRow = getRow(current);
+                    searchRow = mvTable.getTemplateRow();
+                    searchRow.setKey(current.getId());
                 }
             }
             return searchRow;

h2/src/main/org/h2/mvstore/db/MVTable.java

@@ -142,15 +142,8 @@ public class MVTable extends TableBase {
         }
         containsLargeObject = b;
         traceLock = database.getTrace(Trace.LOCK);
-    }
 
-    /**
-     * Initialize the table.
-     *
-     * @param session the session
-     */
-    void init(Session session) {
-        primaryIndex = new MVPrimaryIndex(session.getDatabase(), this, getId(),
+        primaryIndex = new MVPrimaryIndex(database, this, getId(),
                 IndexColumn.wrap(getColumns()), IndexType.createScan(true));
         indexes.add(primaryIndex);
     }

h2/src/main/org/h2/mvstore/db/MVTableEngine.java

@@ -221,7 +221,6 @@ public class MVTableEngine implements TableEngine {
          */
         public MVTable createTable(CreateTableData data) {
             MVTable table = new MVTable(data, this);
-            table.init(data.session);
             tableMap.put(table.getMapName(), table);
             return table;
         }

h2/src/main/org/h2/mvstore/tx/TransactionMap.java

@@ -390,32 +390,6 @@ public class TransactionMap<K, V> {
         return tx == transaction.transactionId;
     }
 
-    /**
-     * Get the versioned value from the raw versioned value (possibly uncommitted),
-     * as visible by the current transaction.
-     *
-     * @param data the value stored in the main map
-     * @param committingTransactions set of transactions being committed
-     *                               at the time when snapshot was taken
-     * @return the value
-     */
-    VersionedValue getValue(VersionedValue data, BitSet committingTransactions) {
-        long id;
-        int tx;
-        // If value doesn't exist or it was deleted by a committed transaction,
-        // or if value is a committed one, just return it.
-        if (data != null &&
-                (id = data.getOperationId()) != 0 &&
-                ((tx = TransactionStore.getTransactionId(id)) != transaction.transactionId &&
-                    !committingTransactions.get(tx))) {
-            // current value comes from another uncommitted transaction
-            // take committed value instead
-            Object committedValue = data.getCommittedValue();
-            data = committedValue == null ? null : VersionedValue.getInstance(committedValue);
-        }
-        return data;
-    }
-
     /**
      * Check whether this map is closed.
      *
@@ -482,23 +456,6 @@ public class TransactionMap<K, V> {
         return it.hasNext() ? it.next() : null;
     }
 
-    /**
-     * Get one of the previous or next keys. There might be no value
-     * available for the returned key.
-     *
-     * @param key the key (may not be null)
-     * @param offset how many keys to skip (-1 for previous, 1 for next)
-     * @return the key
-     */
-    public K relativeKey(K key, long offset) {
-        K k = offset > 0 ? map.ceilingKey(key) : map.floorKey(key);
-        if (k == null) {
-            return k;
-        }
-        long index = map.getKeyIndex(k);
-        return map.getKey(index + offset);
-    }
-
     /**
      * Get the largest key that is smaller than or equal to this key,
      * or null if no such key exists.
@@ -649,15 +606,16 @@ public class TransactionMap<K, V> {
     }
 
     private abstract static class TMIterator<K,X> implements Iterator<X> {
-        private final TransactionMap<K,?> transactionMap;
+        private final int transactionId;
         private final BitSet committingTransactions;
         private final Cursor<K,VersionedValue> cursor;
         private final boolean includeAllUncommitted;
         private X current;
 
         protected TMIterator(TransactionMap<K,?> transactionMap, K from, K to, boolean includeAllUncommitted) {
-            this.transactionMap = transactionMap;
-            TransactionStore store = transactionMap.transaction.store;
+            Transaction transaction = transactionMap.transaction;
+            this.transactionId = transaction.transactionId;
+            TransactionStore store = transaction.store;
             MVMap<K, VersionedValue> map = transactionMap.map;
             // The purpose of the following loop is to get a coherent picture
             // of a state of two independent volatile / atomic variables,
@@ -687,10 +645,23 @@ public class TransactionMap<K, V> {
                 K key = cursor.next();
                 VersionedValue data = cursor.getValue();
                 if (!includeAllUncommitted) {
-                    data = transactionMap.getValue(data, committingTransactions);
+                    // If value doesn't exist or it was deleted by a committed transaction,
+                    // or if value is a committed one, just return it.
+                    if (data != null) {
+                        long id = data.getOperationId();
+                        if (id != 0) {
+                            int tx = TransactionStore.getTransactionId(id);
+                            if (tx != transactionId && !committingTransactions.get(tx)) {
+                                // current value comes from another uncommitted transaction
+                                // take committed value instead
+                                Object committedValue = data.getCommittedValue();
+                                data = committedValue == null ? null : VersionedValue.getInstance(committedValue);
+                            }
+                        }
+                    }
                 }
                 if (data != null && (data.value != null ||
-                        includeAllUncommitted && transactionMap.transaction.transactionId !=
+                        includeAllUncommitted && transactionId !=
                                                     TransactionStore.getTransactionId(data.getOperationId()))) {
                     current = registerCurrent(key, data);
                     return;