Merge pull request #1286 from katzyn/temp_result

Fix MVTempResult implementations for results with invisible columns

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

@@ -5,6 +5,8 @@
  */
 package org.h2.mvstore.db;
 
+import java.util.Arrays;
+
 import org.h2.engine.Database;
 import org.h2.expression.Expression;
 import org.h2.message.DbException;
@@ -14,7 +16,6 @@ import org.h2.mvstore.MVMap.Builder;
 import org.h2.result.ResultExternal;
 import org.h2.value.Value;
 import org.h2.value.ValueArray;
-import org.h2.value.ValueLong;
 
 /**
  * Plain temporary result.
@@ -22,14 +23,14 @@ import org.h2.value.ValueLong;
 class MVPlainTempResult extends MVTempResult {
 
     /**
-     * The type of the values in the main map and keys in the index.
+     * The type of the distinct values.
      */
-    private final ValueDataType valueType;
+    private final ValueDataType distinctType;
 
     /**
      * Map with identities of rows as keys rows as values.
      */
-    private final MVMap<ValueLong, ValueArray> map;
+    private final MVMap<Long, ValueArray> map;
 
     /**
      * Counter for the identities of rows. A separate counter is used instead of
@@ -47,7 +48,7 @@ class MVPlainTempResult extends MVTempResult {
     /**
      * Cursor for the {@link #next()} method.
      */
-    private Cursor<ValueLong, ValueArray> cursor;
+    private Cursor<Long, ValueArray> cursor;
 
     /**
      * Creates a shallow copy of the result.
@@ -57,7 +58,7 @@ class MVPlainTempResult extends MVTempResult {
      */
     private MVPlainTempResult(MVPlainTempResult parent) {
         super(parent);
-        this.valueType = null;
+        this.distinctType = null;
         this.map = parent.map;
     }
 
@@ -65,23 +66,28 @@ class MVPlainTempResult extends MVTempResult {
      * Creates a new plain temporary result.
      *
      * @param database
-     *                        database
+     *            database
      * @param expressions
-     *                        column expressions
+     *            column expressions
+     * @param visibleColumnCount
+     *            count of visible columns
      */
-    MVPlainTempResult(Database database, Expression[] expressions) {
-        super(database);
-        ValueDataType keyType = new ValueDataType(null, null, null);
-        valueType = new ValueDataType(database.getCompareMode(), database, new int[expressions.length]);
-        Builder<ValueLong, ValueArray> builder = new MVMap.Builder<ValueLong, ValueArray>().keyType(keyType)
-                .valueType(valueType);
+    MVPlainTempResult(Database database, Expression[] expressions, int visibleColumnCount) {
+        super(database, expressions.length, visibleColumnCount);
+        ValueDataType valueType = new ValueDataType(database.getCompareMode(), database, new int[columnCount]);
+        if (columnCount == visibleColumnCount) {
+            distinctType = valueType;
+        } else {
+            distinctType = new ValueDataType(database.getCompareMode(), database, new int[visibleColumnCount]);
+        }
+        Builder<Long, ValueArray> builder = new MVMap.Builder<Long, ValueArray>().valueType(valueType);
         map = store.openMap("tmp", builder);
     }
 
     @Override
     public int addRow(Value[] values) {
         assert parent == null && index == null;
-        map.put(ValueLong.get(counter++), ValueArray.get(values));
+        map.put(counter++, ValueArray.get(values));
         return ++rowCount;
     }
 
@@ -98,12 +104,16 @@ class MVPlainTempResult extends MVTempResult {
     }
 
     private void createIndex() {
-        Builder<ValueArray, Boolean> builder = new MVMap.Builder<ValueArray, Boolean>().keyType(valueType);
+        Builder<ValueArray, Boolean> builder = new MVMap.Builder<ValueArray, Boolean>().keyType(distinctType);
         index = store.openMap("idx", builder);
-        Cursor<ValueLong, ValueArray> c = map.cursor(null);
+        Cursor<Long, ValueArray> c = map.cursor(null);
         while (c.hasNext()) {
             c.next();
-            index.putIfAbsent(c.getValue(), true);
+            ValueArray row = c.getValue();
+            if (columnCount != visibleColumnCount) {
+                row = ValueArray.get(Arrays.copyOf(row.getList(), visibleColumnCount));
+            }
+            index.putIfAbsent(row, true);
         }
     }
 

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

@@ -5,10 +5,12 @@
  */
 package org.h2.mvstore.db;
 
+import java.util.Arrays;
 import java.util.BitSet;
 
 import org.h2.engine.Database;
 import org.h2.expression.Expression;
+import org.h2.message.DbException;
 import org.h2.mvstore.Cursor;
 import org.h2.mvstore.MVMap;
 import org.h2.mvstore.MVMap.Builder;
@@ -43,6 +45,19 @@ class MVSortedTempResult extends MVTempResult {
      */
     private final MVMap<ValueArray, Long> map;
 
+    /**
+     * The type of the distinct values.
+     */
+    private final ValueDataType distinctType;
+
+    /**
+     * Optional index. This index is created only if result is distinct and
+     * {@code columnCount != distinctColumnCount} or if
+     * {@link #contains(Value[])} method is invoked. Only the root result should
+     * have an index if required.
+     */
+    private MVMap<ValueArray, Boolean> index;
+
     /**
      * Cursor for the {@link #next()} method.
      */
@@ -71,6 +86,7 @@ class MVSortedTempResult extends MVTempResult {
         this.distinct = parent.distinct;
         this.indexes = parent.indexes;
         this.map = parent.map;
+        this.distinctType = null;
         this.rowCount = parent.rowCount;
     }
 
@@ -78,19 +94,22 @@ class MVSortedTempResult extends MVTempResult {
      * Creates a new sorted temporary result.
      *
      * @param database
-     *                        database
+     *            database
      * @param expressions
-     *                        column expressions
+     *            column expressions
      * @param distinct
-     *                        whether this result should be distinct
+     *            whether this result should be distinct
+     * @param visibleColumnCount
+     *            count of visible columns
      * @param sort
-     *                        sort order, or {@code null} if this result does not
-     *                        need any sorting
+     *            sort order, or {@code null} if this result does not need any
+     *            sorting
      */
-    MVSortedTempResult(Database database, Expression[] expressions, boolean distinct, SortOrder sort) {
-        super(database);
+    MVSortedTempResult(Database database, Expression[] expressions, boolean distinct, int visibleColumnCount,
+            SortOrder sort) {
+        super(database, expressions.length, visibleColumnCount);
         this.distinct = distinct;
-        int length = expressions.length;
+        int length = columnCount;
         int[] sortTypes = new int[length];
         int[] indexes;
         if (sort != null) {
@@ -147,6 +166,14 @@ class MVSortedTempResult extends MVTempResult {
         ValueDataType keyType = new ValueDataType(database.getCompareMode(), database, sortTypes);
         Builder<ValueArray, Long> builder = new MVMap.Builder<ValueArray, Long>().keyType(keyType);
         map = store.openMap("tmp", builder);
+        if (length == visibleColumnCount) {
+            distinctType = null;
+        } else {
+            distinctType = new ValueDataType(database.getCompareMode(), database, new int[visibleColumnCount]);
+            if (distinct) {
+                createIndex(false);
+            }
+        }
     }
 
     @Override
@@ -154,6 +181,12 @@ class MVSortedTempResult extends MVTempResult {
         assert parent == null;
         ValueArray key = getKey(values);
         if (distinct) {
+            if (columnCount != visibleColumnCount) {
+                ValueArray distinctRow = ValueArray.get(Arrays.copyOf(values, visibleColumnCount));
+                if (index.putIfAbsent(distinctRow, true) != null) {
+                    return rowCount;
+                }
+            }
             // Add a row and increment the counter only if row does not exist
             if (map.putIfAbsent(key, 1L) == null) {
                 rowCount++;
@@ -172,9 +205,33 @@ class MVSortedTempResult extends MVTempResult {
 
     @Override
     public boolean contains(Value[] values) {
+        // Only parent result maintains the index
+        if (parent != null) {
+            return parent.contains(values);
+        }
+        if (columnCount != visibleColumnCount) {
+            if (index == null) {
+                createIndex(true);
+            }
+            return index.containsKey(ValueArray.get(values));
+        }
         return map.containsKey(getKey(values));
     }
 
+    private void createIndex(boolean fill) {
+        Builder<ValueArray, Boolean> indexBuilder = new MVMap.Builder<ValueArray, Boolean>()
+                .keyType(distinctType);
+        index = store.openMap("idx", indexBuilder);
+        if (fill) {
+            Cursor<ValueArray, Long> c = map.cursor(null);
+            while (c.hasNext()) {
+                Value[] v = getValue(c.next().getList());
+                ValueArray distinctRow = ValueArray.get(Arrays.copyOf(v, visibleColumnCount));
+                index.putIfAbsent(distinctRow, true);
+            }
+        }
+    }
+
     @Override
     public synchronized ResultExternal createShallowCopy() {
         if (parent != null) {
@@ -198,7 +255,7 @@ class MVSortedTempResult extends MVTempResult {
         if (indexes != null) {
             Value[] r = new Value[indexes.length];
             for (int i = 0; i < indexes.length; i++) {
-                r[indexes[i]] = values[i];
+                r[i] = values[indexes[i]];
             }
             values = r;
         }
@@ -216,7 +273,7 @@ class MVSortedTempResult extends MVTempResult {
         if (indexes != null) {
             Value[] r = new Value[indexes.length];
             for (int i = 0; i < indexes.length; i++) {
-                r[i] = key[indexes[i]];
+                r[indexes[i]] = key[i];
             }
             key = r;
         }
@@ -256,6 +313,9 @@ class MVSortedTempResult extends MVTempResult {
     @Override
     public int removeRow(Value[] values) {
         assert parent == null && distinct;
+        if (columnCount != visibleColumnCount) {
+            throw DbException.getUnsupportedException("removeRow()");
+        }
         // If an entry was removed decrement the counter
         if (map.remove(getKey(values)) != null) {
             rowCount--;

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

@@ -61,18 +61,22 @@ public abstract class MVTempResult implements ResultExternal {
      * Creates MVStore-based temporary result.
      *
      * @param database
-     *                        database
+     *            database
      * @param expressions
-     *                        expressions
+     *            expressions
      * @param distinct
-     *                        is output distinct
+     *            is output distinct
+     * @param visibleColumnCount
+     *            count of visible columns
      * @param sort
-     *                        sort order, or {@code null}
+     *            sort order, or {@code null}
      * @return temporary result
      */
-    public static ResultExternal of(Database database, Expression[] expressions, boolean distinct, SortOrder sort) {
-        return distinct || sort != null ? new MVSortedTempResult(database, expressions, distinct, sort)
-                : new MVPlainTempResult(database, expressions);
+    public static ResultExternal of(Database database, Expression[] expressions, boolean distinct,
+            int visibleColumnCount, SortOrder sort) {
+        return distinct || sort != null
+                ? new MVSortedTempResult(database, expressions, distinct, visibleColumnCount, sort)
+                : new MVPlainTempResult(database, expressions, visibleColumnCount);
     }
 
     /**
@@ -80,6 +84,16 @@ public abstract class MVTempResult implements ResultExternal {
      */
     final MVStore store;
 
+    /**
+     * Count of columns.
+     */
+    final int columnCount;
+
+    /**
+     * Count of visible columns.
+     */
+    final int visibleColumnCount;
+
     /**
      * Count of rows. Used only in a root results, copies always have 0 value.
      */
@@ -124,6 +138,8 @@ public abstract class MVTempResult implements ResultExternal {
     MVTempResult(MVTempResult parent) {
         this.parent = parent;
         this.store = parent.store;
+        this.columnCount = parent.columnCount;
+        this.visibleColumnCount = parent.visibleColumnCount;
         this.tempFileDeleter = null;
         this.closeable = null;
         this.fileRef = null;
@@ -133,9 +149,13 @@ public abstract class MVTempResult implements ResultExternal {
      * Creates a new temporary result.
      *
      * @param database
-     *                     database
+     *            database
+     * @param columnCount
+     *            count of columns
+     * @param visibleColumnCount
+     *            count of visible columns
      */
-    MVTempResult(Database database) {
+    MVTempResult(Database database, int columnCount, int visibleColumnCount) {
         try {
             String fileName = FileUtils.createTempFile("h2tmp", Constants.SUFFIX_TEMP_FILE, false, true);
             Builder builder = new MVStore.Builder().fileName(fileName);
@@ -144,6 +164,8 @@ public abstract class MVTempResult implements ResultExternal {
                 builder.encryptionKey(MVTableEngine.decodePassword(key));
             }
             store = builder.open();
+            this.columnCount = columnCount;
+            this.visibleColumnCount = visibleColumnCount;
             tempFileDeleter = database.getTempFileDeleter();
             closeable = new CloseImpl(store, fileName);
             fileRef = tempFileDeleter.addFile(closeable, this);

h2/src/main/org/h2/result/LocalResult.java

@@ -192,6 +192,7 @@ public class LocalResult implements ResultInterface, ResultTarget {
         if (!distinct) {
             DbException.throwInternalError();
         }
+        assert values.length == visibleColumnCount;
         if (distinctRows != null) {
             ValueArray array = ValueArray.get(values);
             distinctRows.remove(array);
@@ -209,6 +210,7 @@ public class LocalResult implements ResultInterface, ResultTarget {
      */
     @Override
     public boolean containsDistinct(Value[] values) {
+        assert values.length == visibleColumnCount;
         if (external != null) {
             return external.contains(values);
         }
@@ -286,7 +288,7 @@ public class LocalResult implements ResultInterface, ResultTarget {
     private void createExternalResult() {
         Database database = session.getDatabase();
         external = database.isMVStore()
-                ? MVTempResult.of(database, expressions, distinct, sort)
+                ? MVTempResult.of(database, expressions, distinct, visibleColumnCount, sort)
                         : new ResultTempTable(session, expressions, distinct, sort);
     }
 

h2/src/test/org/h2/test/scripts/testScript.sql

@@ -3077,17 +3077,17 @@ SELECT t.f1, t.f2 FROM test t ORDER BY t.f2;
 > abc 333
 > rows (ordered): 3
 
-SELECT t1.f1, t1.f2, t2.f1, t2.f2 FROM test t1, test t2 ORDER BY t2.f2;
+SELECT t1.f1, t1.f2, t2.f1, t2.f2 FROM test t1, test t2 ORDER BY t2.f2, t1.f2;
 > F1  F2  F1  F2
 > --- --- --- ---
-> abc 222 abc 111
 > abc 111 abc 111
+> abc 222 abc 111
 > abc 333 abc 111
-> abc 222 abc 222
 > abc 111 abc 222
+> abc 222 abc 222
 > abc 333 abc 222
-> abc 222 abc 333
 > abc 111 abc 333
+> abc 222 abc 333
 > abc 333 abc 333
 > rows (ordered): 9