Merge pull request #1 from h2database/master

Fork Sync

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

@@ -12,10 +12,16 @@ import org.h2.api.ErrorCode;
 import org.h2.command.Prepared;
 import org.h2.engine.Database;
 import org.h2.engine.Session;
+import org.h2.engine.Mode.ModeEnum;
 import org.h2.expression.Alias;
+import org.h2.expression.Comparison;
+import org.h2.expression.ConditionAndOr;
+import org.h2.expression.ConditionNot;
 import org.h2.expression.Expression;
 import org.h2.expression.ExpressionColumn;
 import org.h2.expression.ExpressionVisitor;
+import org.h2.expression.Function;
+import org.h2.expression.Operation;
 import org.h2.expression.Parameter;
 import org.h2.expression.ValueExpression;
 import org.h2.message.DbException;
@@ -478,8 +484,11 @@ public abstract class Query extends Prepared {
             }
             if (!isAlias) {
                 if (mustBeInResult) {
-                    throw DbException.get(ErrorCode.ORDER_BY_NOT_IN_RESULT,
-                            e.getSQL());
+                    if (session.getDatabase().getMode().getEnum() != ModeEnum.MySQL) {
+                        if (!checkOrderOther(session, e, expressionSQL)) {
+                            throw DbException.get(ErrorCode.ORDER_BY_NOT_IN_RESULT, e.getSQL());
+                        }
+                    }
                 }
                 expressions.add(e);
                 String sql = e.getSQL();
@@ -490,6 +499,64 @@ public abstract class Query extends Prepared {
         }
     }
 
+    /**
+     * An additional check for expression in ORDER BY list for DISTINCT selects
+     * that was not matched with selected expressions in regular way. This
+     * method allows expressions based only on selected expressions in different
+     * complicated ways with functions, comparisons, or operators.
+     *
+     * @param session session
+     * @param expr expression to check
+     * @param expressionSQL SQL of allowed expressions
+     * @return whether the specified expression should be allowed in ORDER BY
+     *         list of DISTINCT select
+     */
+    private static boolean checkOrderOther(Session session, Expression expr, ArrayList<String> expressionSQL) {
+        if (expr.isConstant()) {
+            return true;
+        }
+        if (expressionSQL != null) {
+            String exprSQL = expr.getSQL();
+            for (String sql: expressionSQL) {
+                if (session.getDatabase().equalsIdentifiers(exprSQL, sql)) {
+                    return true;
+                }
+            }
+        }
+        if (expr instanceof Function) {
+            Function function = (Function) expr;
+            if (!function.isDeterministic()) {
+                return false;
+            }
+            for (Expression e : function.getArgs()) {
+                if (!checkOrderOther(session, e, expressionSQL)) {
+                    return false;
+                }
+            }
+            return true;
+        }
+        if (expr instanceof Operation) {
+            Operation operation = (Operation) expr;
+            Expression right = operation.getRightSubExpression();
+            return checkOrderOther(session, operation.getLeftSubExpression(), expressionSQL)
+                    && (right == null || checkOrderOther(session, right, expressionSQL));
+        }
+        if (expr instanceof ConditionAndOr) {
+            ConditionAndOr condition = (ConditionAndOr) expr;
+            return checkOrderOther(session, condition.getLeftSubExpression(), expressionSQL)
+                    && checkOrderOther(session, condition.getRightSubExpression(), expressionSQL);
+        }
+        if (expr instanceof ConditionNot) {
+            return checkOrderOther(session, ((ConditionNot) expr).getSubCondition(), expressionSQL);
+        }
+        if (expr instanceof Comparison) {
+            Comparison condition = (Comparison) expr;
+            return checkOrderOther(session, condition.getLeftSubExpression(), expressionSQL)
+                    && checkOrderOther(session, condition.getRightSubExpression(), expressionSQL);
+        }
+        return false;
+    }
+
     /**
      * Create a {@link SortOrder} object given the list of {@link SelectOrderBy}
      * objects. The expression list is extended if necessary.

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

@@ -597,14 +597,21 @@ public class Comparison extends Condition {
     }
 
     /**
-     * Get the left or the right sub-expression of this condition.
+     * Get the left sub-expression of this condition.
      *
-     * @param getLeft true to get the left sub-expression, false to get the
-     *            right sub-expression.
-     * @return the sub-expression
+     * @return the left sub-expression
      */
-    public Expression getExpression(boolean getLeft) {
-        return getLeft ? this.left : right;
+    public Expression getLeftSubExpression() {
+        return left;
+    }
+
+    /**
+     * Get the right sub-expression of this condition.
+     *
+     * @return the right sub-expression
+     */
+    public Expression getRightSubExpression() {
+        return right;
     }
 
 }

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

@@ -284,14 +284,21 @@ public class ConditionAndOr extends Condition {
     }
 
     /**
-     * Get the left or the right sub-expression of this condition.
+     * Get the left sub-expression of this condition.
      *
-     * @param getLeft true to get the left sub-expression, false to get the
-     *            right sub-expression.
-     * @return the sub-expression
+     * @return the left sub-expression
      */
-    public Expression getExpression(boolean getLeft) {
-        return getLeft ? this.left : right;
+    public Expression getLeftSubExpression() {
+        return left;
+    }
+
+    /**
+     * Get the right sub-expression of this condition.
+     *
+     * @return the right sub-expression
+     */
+    public Expression getRightSubExpression() {
+        return right;
     }
 
 }

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

@@ -98,4 +98,13 @@ public class ConditionNot extends Condition {
         return condition.getCost();
     }
 
+    /**
+     * Get the sub-expression of this condition.
+     *
+     * @return the sub-expression
+     */
+    public Expression getSubCondition() {
+        return condition;
+    }
+
 }

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

@@ -407,4 +407,22 @@ public class Operation extends Expression {
         return left.getCost() + 1 + (right == null ? 0 : right.getCost());
     }
 
+    /**
+     * Get the left sub-expression of this operation.
+     *
+     * @return the left sub-expression
+     */
+    public Expression getLeftSubExpression() {
+        return left;
+    }
+
+    /**
+     * Get the right sub-expression of this operation.
+     *
+     * @return the right sub-expression
+     */
+    public Expression getRightSubExpression() {
+        return right;
+    }
+
 }

h2/src/main/org/h2/fulltext/FullText.java

@@ -669,14 +669,14 @@ public class FullText {
             ArrayList<String> data, Expression expr) {
         if (expr instanceof ConditionAndOr) {
             ConditionAndOr and = (ConditionAndOr) expr;
-            Expression left = and.getExpression(true);
-            Expression right = and.getExpression(false);
+            Expression left = and.getLeftSubExpression();
+            Expression right = and.getRightSubExpression();
             addColumnData(columns, data, left);
             addColumnData(columns, data, right);
         } else {
             Comparison comp = (Comparison) expr;
-            ExpressionColumn ec = (ExpressionColumn) comp.getExpression(true);
-            ValueExpression ev = (ValueExpression) comp.getExpression(false);
+            ExpressionColumn ec = (ExpressionColumn) comp.getLeftSubExpression();
+            ValueExpression ev = (ValueExpression) comp.getRightSubExpression();
             String columnName = ec.getColumnName();
             columns.add(columnName);
             if (ev == null) {

h2/src/main/org/h2/message/TraceObject.java

@@ -105,6 +105,8 @@ public class TraceObject {
             "rs", "rsMeta", "sp", "ex", "stat", "blob", "clob", "pMeta", "ds",
             "xads", "xares", "xid", "ar", "sqlxml" };
 
+    private static final SQLException SQL_OOME = DbException.SQL_OOME;
+
     /**
      * The trace module used by this object.
      */
@@ -376,7 +378,7 @@ public class TraceObject {
                 try {
                     e = new SQLException("GeneralError", "HY000", ErrorCode.GENERAL_ERROR_1, ex);
                 } catch (OutOfMemoryError | NoClassDefFoundError ignored) {
-                    return DbException.SQL_OOME;
+                    return SQL_OOME;
                 }
             }
             e.addSuppressed(another);

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;
         }
@@ -255,26 +312,13 @@ class MVSortedTempResult extends MVTempResult {
 
     @Override
     public int removeRow(Value[] values) {
-        assert parent == null;
-        ValueArray key = getKey(values);
-        if (distinct) {
-            // If an entry was removed decrement the counter
-            if (map.remove(key) != null) {
-                rowCount--;
-            }
-        } else {
-            Long old = map.remove(key);
-            if (old != null) {
-                long l = old;
-                if (l > 1) {
-                    /*
-                     * We have more than one such row. Decrement its counter by 1 and put this row
-                     * back into map.
-                     */
-                    map.put(key, l - 1);
-                }
-                rowCount--;
-            }
+        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--;
         }
         return 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);
@@ -186,9 +208,4 @@ public abstract class MVTempResult implements ResultExternal {
         tempFileDeleter.deleteFile(fileRef, closeable);
     }
 
-    @Override
-    public void done() {
-        // Do nothing
-    }
-
 }

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

@@ -42,7 +42,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
     private int offset;
     private int limit = -1;
     private ResultExternal external;
-    private int diskOffset;
     private boolean distinct;
     private boolean randomAccess;
     private boolean closed;
@@ -157,7 +156,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
         copy.offset = 0;
         copy.limit = -1;
         copy.external = e2;
-        copy.diskOffset = this.diskOffset;
         return copy;
     }
 
@@ -194,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);
@@ -211,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);
         }
@@ -231,11 +231,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
         currentRow = null;
         if (external != null) {
             external.reset();
-            if (diskOffset > 0) {
-                for (int i = 0; i < diskOffset; i++) {
-                    external.next();
-                }
-            }
         }
     }
 
@@ -293,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);
     }
 
@@ -318,19 +313,19 @@ public class LocalResult implements ResultInterface, ResultTarget {
             } else {
                 rowCount = external.addRow(values);
             }
-            return;
-        }
-        rows.add(values);
-        rowCount++;
-        if (rows.size() > maxMemoryRows) {
-            if (external == null) {
-                createExternalResult();
+        } else {
+            rows.add(values);
+            rowCount++;
+            if (rows.size() > maxMemoryRows) {
+                addRowsToDisk();
             }
-            addRowsToDisk();
         }
     }
 
     private void addRowsToDisk() {
+        if (external == null) {
+            createExternalResult();
+        }
         rowCount = external.addRows(rows);
         rows.clear();
     }
@@ -344,41 +339,12 @@ public class LocalResult implements ResultInterface, ResultTarget {
      * This method is called after all rows have been added.
      */
     public void done() {
-        if (distinct) {
-            if (distinctRows != null) {
-                rows = distinctRows.values();
-            } else {
-                if (external != null && sort != null) {
-                    // external sort
-                    ResultExternal temp = external;
-                    external = null;
-                    temp.reset();
-                    rows = Utils.newSmallArrayList();
-                    // TODO use offset directly if possible
-                    while (true) {
-                        Value[] list = temp.next();
-                        if (list == null) {
-                            break;
-                        }
-                        if (external == null) {
-                            createExternalResult();
-                        }
-                        rows.add(list);
-                        if (rows.size() > maxMemoryRows) {
-                            rowCount = external.addRows(rows);
-                            rows.clear();
-                        }
-                    }
-                    temp.close();
-                    // the remaining data in rows is written in the following
-                    // lines
-                }
-            }
-        }
         if (external != null) {
             addRowsToDisk();
-            external.done();
         } else {
+            if (distinct) {
+                rows = distinctRows.values();
+            }
             if (sort != null) {
                 if (offset > 0 || limit > 0) {
                     sort.sort(rows, offset, limit < 0 ? rows.size() : limit);
@@ -387,11 +353,64 @@ public class LocalResult implements ResultInterface, ResultTarget {
                 }
             }
         }
-        applyOffset();
-        applyLimit();
+        applyOffsetAndLimit();
         reset();
     }
 
+    private void applyOffsetAndLimit() {
+        int offset = Math.max(this.offset, 0);
+        int limit = this.limit;
+        if (offset == 0 && limit < 0 || rowCount == 0) {
+            return;
+        }
+        boolean clearAll = offset >= rowCount || limit == 0;
+        if (!clearAll) {
+            int remaining = rowCount - offset;
+            limit = limit < 0 ? remaining : Math.min(remaining, limit);
+            if (offset == 0 && remaining <= limit) {
+                return;
+            }
+        } else {
+            limit = 0;
+        }
+        distinctRows = null;
+        rowCount = limit;
+        if (external == null) {
+            if (clearAll) {
+                rows.clear();
+                return;
+            }
+            // avoid copying the whole array for each row
+            rows = new ArrayList<>(rows.subList(offset, offset + limit));
+        } else {
+            if (clearAll) {
+                external.close();
+                external = null;
+                return;
+            }
+            trimExternal(offset, limit);
+        }
+    }
+
+    private void trimExternal(int offset, int limit) {
+        ResultExternal temp = external;
+        external = null;
+        temp.reset();
+        while (--offset >= 0) {
+            temp.next();
+        }
+        while (--limit >= 0) {
+            rows.add(temp.next());
+            if (rows.size() > maxMemoryRows) {
+                addRowsToDisk();
+            }
+        }
+        if (external != null) {
+            addRowsToDisk();
+        }
+        temp.close();
+    }
+
     @Override
     public int getRowCount() {
         return rowCount;
@@ -411,24 +430,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
         this.limit = limit;
     }
 
-    private void applyLimit() {
-        if (limit < 0) {
-            return;
-        }
-        if (external == null) {
-            if (rows.size() > limit) {
-                rows = new ArrayList<>(rows.subList(0, limit));
-                rowCount = limit;
-                distinctRows = null;
-            }
-        } else {
-            if (limit < rowCount) {
-                rowCount = limit;
-                distinctRows = null;
-            }
-        }
-    }
-
     @Override
     public boolean needToClose() {
         return external != null;
@@ -502,31 +503,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
         this.offset = offset;
     }
 
-    private void applyOffset() {
-        if (offset <= 0) {
-            return;
-        }
-        if (external == null) {
-            if (offset >= rows.size()) {
-                rows.clear();
-                rowCount = 0;
-            } else {
-                // avoid copying the whole array for each row
-                int remove = Math.min(offset, rows.size());
-                rows = new ArrayList<>(rows.subList(remove, rows.size()));
-                rowCount -= remove;
-            }
-        } else {
-            if (offset >= rowCount) {
-                rowCount = 0;
-            } else {
-                diskOffset = offset;
-                rowCount -= offset;
-            }
-        }
-        distinctRows = null;
-    }
-
     @Override
     public String toString() {
         return super.toString() + " columns: " + visibleColumnCount +

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

@@ -42,11 +42,6 @@ public interface ResultExternal {
      */
     int addRows(ArrayList<Value[]> rows);
 
-    /**
-     * This method is called after all rows have been added.
-     */
-    void done();
-
     /**
      * Close this object and delete the temporary file.
      */

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

@@ -309,11 +309,6 @@ public class ResultTempTable implements ResultExternal {
         }
     }
 
-    @Override
-    public void done() {
-        // nothing to do
-    }
-
     @Override
     public Value[] next() {
         if (resultCursor == null) {

h2/src/main/org/h2/store/Data.java

@@ -738,7 +738,7 @@ public class Data {
         case Value.TIMESTAMP: {
             return ValueTimestamp.fromMillisNanos(
                     DateTimeUtils.getTimeUTCWithoutDst(readVarLong()),
-                    readVarInt());
+                    readVarInt() % 1_000_000);
         }
         case Value.TIMESTAMP_TZ: {
             long dateValue = readVarLong();

h2/src/test/org/h2/test/scripts/TestScript.java

@@ -99,6 +99,7 @@ public class TestScript extends TestDb {
         testScript("testScript.sql");
         testScript("comments.sql");
         testScript("derived-column-names.sql");
+        testScript("distinct.sql");
         testScript("dual.sql");
         testScript("indexes.sql");
         testScript("information_schema.sql");

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

+-- Copyright 2004-2018 H2 Group. Multiple-Licensed under the MPL 2.0,
+-- and the EPL 1.0 (http://h2database.com/html/license.html).
+-- Initial Developer: H2 Group
+--
+
+CREATE TABLE TEST(ID BIGINT, NAME VARCHAR);
+> ok
+
+INSERT INTO TEST VALUES (1, 'a'), (2, 'B'), (3, 'c'), (1, 'a');
+> update count: 4
+
+CREATE TABLE TEST2(ID2 BIGINT);
+> ok
+
+INSERT INTO TEST2 VALUES (1), (2);
+> update count: 2
+
+SELECT DISTINCT NAME FROM TEST ORDER BY NAME;
+> NAME
+> ----
+> B
+> a
+> c
+> rows (ordered): 3
+
+SELECT DISTINCT NAME FROM TEST ORDER BY LOWER(NAME);
+> NAME
+> ----
+> a
+> B
+> c
+> rows (ordered): 3
+
+SELECT DISTINCT ID FROM TEST ORDER BY ID;
+> ID
+> --
+> 1
+> 2
+> 3
+> rows (ordered): 3
+
+SELECT DISTINCT ID FROM TEST ORDER BY -ID - 1;
+> ID
+> --
+> 3
+> 2
+> 1
+> rows (ordered): 3
+
+SELECT DISTINCT ID FROM TEST ORDER BY (-ID + 10) > 0 AND NOT (ID = 0), ID;
+> ID
+> --
+> 1
+> 2
+> 3
+> rows (ordered): 3
+
+SELECT DISTINCT NAME, ID + 1 FROM TEST ORDER BY UPPER(NAME) || (ID + 1);
+> NAME ID + 1
+> ---- ------
+> a    2
+> B    3
+> c    4
+> rows (ordered): 3
+
+SELECT DISTINCT ID FROM TEST ORDER BY NAME;
+> exception ORDER_BY_NOT_IN_RESULT
+
+SELECT DISTINCT ID FROM TEST ORDER BY CURRENT_TIMESTAMP;
+> exception ORDER_BY_NOT_IN_RESULT
+
+SET MODE MySQL;
+> ok
+
+SELECT DISTINCT ID FROM TEST ORDER BY NAME;
+> ID
+> --
+> 2
+> 1
+> 3
+> rows (ordered): 3
+
+SELECT DISTINCT ID FROM TEST ORDER BY LOWER(NAME);
+> ID
+> --
+> 1
+> 2
+> 3
+> rows (ordered): 3
+
+SELECT DISTINCT ID FROM TEST JOIN TEST2 ON ID = ID2 ORDER BY LOWER(NAME);
+> ID
+> --
+> 1
+> 2
+> rows (ordered): 2
+
+SET MODE Regular;
+> ok
+
+DROP TABLE TEST;
+> ok
+
+DROP TABLE TEST2;
+> ok

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