Merge pull request #1287 from katzyn/distinct

DISTINCT does not work with ORDER BY in some queries

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/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