Merge pull request #1299 from katzyn/distinct

Handle distinct results properly

h2/src/main/org/h2/command/Parser.java

@@ -2535,7 +2535,7 @@ public class Parser {
         }
         currentSelect = temp;
         if (readIf(DISTINCT)) {
-            command.setDistinct(true);
+            command.setDistinct();
         } else {
             readIf(ALL);
         }
@@ -2749,9 +2749,7 @@ public class Parser {
                 } else {
                     if (isSelect()) {
                         Query query = parseSelect();
-                        // can not be lazy because we have to call
-                        // method ResultInterface.containsDistinct
-                        // which is not supported for lazy execution
+                        // TODO lazy result causes timeout in TestFuzzOptimizations
                         query.setNeverLazy(true);
                         r = new ConditionInSelect(database, r, query, false,
                                 Comparison.EQUAL);

h2/src/main/org/h2/command/dml/Query.java

@@ -255,13 +255,17 @@ public abstract class Query extends Prepared {
 
     /**
      * Set the distinct flag.
-     *
-     * @param b the new value
      */
-    public void setDistinct(boolean b) {
-        distinct = b;
+    public void setDistinct() {
+        distinct = true;
     }
 
+    /**
+     * Set the distinct flag only if it is possible, may be used as a possible
+     * optimization only.
+     */
+    public abstract void setDistinctIfPossible();
+
     public boolean isDistinct() {
         return distinct;
     }

h2/src/main/org/h2/command/dml/Select.java

@@ -247,6 +247,13 @@ public class Select extends Query {
         return orderList != null || sort != null;
     }
 
+    @Override
+    public void setDistinctIfPossible() {
+        if (!distinct && offsetExpr == null && limitExpr == null) {
+            setDistinct();
+        }
+    }
+
     /**
      * Add a condition to the list of conditions.
      *
@@ -667,9 +674,6 @@ public class Select extends Query {
             result = createLocalResult(result);
             result.setDistinct();
         }
-        if (randomAccessResult) {
-            result = createLocalResult(result);
-        }
         if (isGroupQuery && !isGroupSortedQuery) {
             result = createLocalResult(result);
         }
@@ -724,8 +728,12 @@ public class Select extends Query {
             if (limitRows > 0) {
                 lazyResult.setLimit(limitRows);
             }
+            if (randomAccessResult) {
+                return convertToDistinct(lazyResult);
+            } else {
                 return lazyResult;
             }
+        }
         if (offsetExpr != null) {
             result.setOffset(offsetExpr.getValue(session).getInt());
         }
@@ -734,6 +742,9 @@ public class Select extends Query {
         }
         if (result != null) {
             result.done();
+            if (randomAccessResult && !distinct) {
+                result = convertToDistinct(result);
+            }
             if (target != null) {
                 while (result.next()) {
                     target.addRow(result.currentRow());
@@ -761,6 +772,18 @@ public class Select extends Query {
                 visibleColumnCount);
     }
 
+    private LocalResult convertToDistinct(ResultInterface result) {
+        LocalResult distinctResult = new LocalResult(session, expressionArray, visibleColumnCount);
+        distinctResult.setDistinct();
+        result.reset();
+        while (result.next()) {
+            distinctResult.addRow(result.currentRow());
+        }
+        result.close();
+        distinctResult.done();
+        return distinctResult;
+    }
+
     private void expandColumnList() {
         Database db = session.getDatabase();
 

h2/src/main/org/h2/command/dml/SelectUnion.java

@@ -125,6 +125,11 @@ public class SelectUnion extends Query {
         return orderList != null || sort != null;
     }
 
+    @Override
+    public void setDistinctIfPossible() {
+        setDistinct();
+    }
+
     private Value[] convert(Value[] values, int columnCount) {
         Value[] newValues;
         if (columnCount == values.length) {
@@ -210,25 +215,22 @@ public class SelectUnion extends Query {
             result.setSortOrder(sort);
         }
         if (distinct) {
-            left.setDistinct(true);
-            right.setDistinct(true);
+            left.setDistinctIfPossible();
+            right.setDistinctIfPossible();
             result.setDistinct();
         }
-        if (randomAccessResult) {
-            result.setRandomAccess();
-        }
         switch (unionType) {
         case UNION:
         case EXCEPT:
-            left.setDistinct(true);
-            right.setDistinct(true);
+            left.setDistinctIfPossible();
+            right.setDistinctIfPossible();
             result.setDistinct();
             break;
         case UNION_ALL:
             break;
         case INTERSECT:
-            left.setDistinct(true);
-            right.setDistinct(true);
+            left.setDistinctIfPossible();
+            right.setDistinctIfPossible();
             break;
         default:
             DbException.throwInternalError("type=" + unionType);
@@ -260,7 +262,6 @@ public class SelectUnion extends Query {
         case INTERSECT: {
             LocalResult temp = new LocalResult(session, expressionArray, columnCount);
             temp.setDistinct();
-            temp.setRandomAccess();
             while (l.next()) {
                 temp.addRow(convert(l.currentRow(), columnCount));
             }

h2/src/main/org/h2/expression/ConditionInSelect.java

@@ -43,9 +43,7 @@ public class ConditionInSelect extends Condition {
     @Override
     public Value getValue(Session session) {
         query.setSession(session);
-        if (!query.hasOrder()) {
-            query.setDistinct(true);
-        }
+        query.setDistinctIfPossible();
         ResultInterface rows = query.query(0);
         Value l = left.getValue(session);
         if (!rows.hasNext()) {

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

@@ -5,8 +5,6 @@
  */
 package org.h2.mvstore.db;
 
-import java.util.Arrays;
-
 import org.h2.engine.Database;
 import org.h2.expression.Expression;
 import org.h2.message.DbException;
@@ -22,11 +20,6 @@ import org.h2.value.ValueArray;
  */
 class MVPlainTempResult extends MVTempResult {
 
-    /**
-     * The type of the distinct values.
-     */
-    private final ValueDataType distinctType;
-
     /**
      * Map with identities of rows as keys rows as values.
      */
@@ -39,12 +32,6 @@ class MVPlainTempResult extends MVTempResult {
      */
     private long counter;
 
-    /**
-     * Optional index. This index is created only 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.
      */
@@ -58,7 +45,6 @@ class MVPlainTempResult extends MVTempResult {
      */
     private MVPlainTempResult(MVPlainTempResult parent) {
         super(parent);
-        this.distinctType = null;
         this.map = parent.map;
     }
 
@@ -75,46 +61,20 @@ class MVPlainTempResult extends MVTempResult {
     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;
+        assert parent == null;
         map.put(counter++, ValueArray.get(values));
         return ++rowCount;
     }
 
     @Override
     public boolean contains(Value[] values) {
-        // Only parent result maintains the index
-        if (parent != null) {
-            return parent.contains(values);
-        }
-        if (index == null) {
-            createIndex();
-        }
-        return index.containsKey(ValueArray.get(values));
-    }
-
-    private void createIndex() {
-        Builder<ValueArray, Boolean> builder = new MVMap.Builder<ValueArray, Boolean>().keyType(distinctType);
-        index = store.openMap("idx", builder);
-        Cursor<Long, ValueArray> c = map.cursor(null);
-        while (c.hasNext()) {
-            c.next();
-            ValueArray row = c.getValue();
-            if (columnCount != visibleColumnCount) {
-                row = ValueArray.get(Arrays.copyOf(row.getList(), visibleColumnCount));
-            }
-            index.putIfAbsent(row, true);
-        }
+        throw DbException.getUnsupportedException("contains()");
     }
 
     @Override

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

@@ -171,7 +171,9 @@ class MVSortedTempResult extends MVTempResult {
         } else {
             distinctType = new ValueDataType(database.getCompareMode(), database, new int[visibleColumnCount]);
             if (distinct) {
-                createIndex(false);
+                Builder<ValueArray, Boolean> indexBuilder = new MVMap.Builder<ValueArray, Boolean>()
+                        .keyType(distinctType);
+                index = store.openMap("idx", indexBuilder);
             }
         }
     }
@@ -209,29 +211,13 @@ class MVSortedTempResult extends MVTempResult {
         if (parent != null) {
             return parent.contains(values);
         }
+        assert distinct;
         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) {

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

@@ -43,7 +43,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
     private int limit = -1;
     private ResultExternal external;
     private boolean distinct;
-    private boolean randomAccess;
     private boolean closed;
     private boolean containsLobs;
 
@@ -151,7 +150,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
         copy.sort = this.sort;
         copy.distinctRows = this.distinctRows;
         copy.distinct = distinct;
-        copy.randomAccess = randomAccess;
         copy.currentRow = null;
         copy.offset = 0;
         copy.limit = -1;
@@ -176,13 +174,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
         distinctRows = ValueHashMap.newInstance();
     }
 
-    /**
-     * Random access is required (containsDistinct).
-     */
-    public void setRandomAccess() {
-        this.randomAccess = true;
-    }
-
     /**
      * Remove the row from the result set if it exists.
      *
@@ -288,6 +279,7 @@ public class LocalResult implements ResultInterface, ResultTarget {
     private void createExternalResult() {
         Database database = session.getDatabase();
         external = database.isMVStore()
+                || /* not supported by ResultTempTable */ distinct && expressions.length != visibleColumnCount
                 ? MVTempResult.of(database, expressions, distinct, visibleColumnCount, sort)
                         : new ResultTempTable(session, expressions, distinct, sort);
     }

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

@@ -144,36 +144,7 @@ public class ResultTempTable implements ResultExternal {
             closeable = new CloseImpl(session, table);
             fileRef = tempFileDeleter.addFile(closeable, this);
         }
-        /*
-         * If ORDER BY or DISTINCT is specified create the index immediately. If
-         * they are not specified index still may be created later if required
-         * for IN (SELECT ...) etc.
-         */
         if (sort != null || distinct) {
-            getIndex();
-        }
-    }
-
-    private ResultTempTable(ResultTempTable parent) {
-        this.parent = parent;
-        this.columnCount = parent.columnCount;
-        this.distinct = parent.distinct;
-        this.session = parent.session;
-        this.table = parent.table;
-        this.rowCount = parent.rowCount;
-        this.sort = parent.sort;
-        this.tempFileDeleter = null;
-        this.closeable = null;
-        this.fileRef = null;
-    }
-
-    private Index getIndex() {
-        if (parent != null) {
-            return parent.getIndex();
-        }
-        if (index != null) {
-            return index;
-        }
             IndexColumn[] indexCols;
             if (sort != null) {
                 int[] colIndex = sort.getQueryColumnIndexes();
@@ -215,6 +186,26 @@ public class ResultTempTable implements ResultExternal {
             if (closeable != null) {
                 closeable.index = index;
             }
+        }
+    }
+
+    private ResultTempTable(ResultTempTable parent) {
+        this.parent = parent;
+        this.columnCount = parent.columnCount;
+        this.distinct = parent.distinct;
+        this.session = parent.session;
+        this.table = parent.table;
+        this.rowCount = parent.rowCount;
+        this.sort = parent.sort;
+        this.tempFileDeleter = null;
+        this.closeable = null;
+        this.fileRef = null;
+    }
+
+    private Index getIndex() {
+        if (parent != null) {
+            return parent.getIndex();
+        }
         return index;
     }