In version 1.4.186, order by was broken in some cases due to the change Make the…

In version 1.4.186, order by was broken in some cases due to the change Make the planner use indexes for sorting when doing a GROUP BY. The change was reverted.

h2/src/docsrc/html/changelog.html

@@ -21,6 +21,10 @@ Change Log
 
 <h2>Next Version (unreleased)</h2>
 <ul>
+<li>In version 1.4.186, "order by" was broken in some cases
+    due to the change "Make the planner use indexes for sorting when doing a GROUP BY".
+    The change was reverted.
+</li>
 <li>Pull request #146: Improved CompareMode.
 </li>
 <li>Fix for #144, JdbcResultSet.setFetchDirection() throws "Feature not supported".

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

@@ -25,7 +25,6 @@ import org.h2.expression.ExpressionVisitor;
 import org.h2.expression.Parameter;
 import org.h2.index.Cursor;
 import org.h2.index.Index;
-import org.h2.index.IndexCondition;
 import org.h2.index.IndexType;
 import org.h2.message.DbException;
 import org.h2.result.LocalResult;
@@ -445,16 +444,11 @@ public class Select extends Query {
                     // with the exact same columns
                     IndexColumn idxCol = indexCols[j];
                     Column sortCol = sortCols[j];
-                    boolean implicitSortColumn = false;
                     if (idxCol.column != sortCol) {
-                        implicitSortColumn = isSortColumnImplicit(
-                                topTableFilter, idxCol.column);
-                        if (!implicitSortColumn) {
                         ok = false;
                         break;
                     }
-                    }
-                    if (!implicitSortColumn && idxCol.sortType != sortTypes[j]) {
+                    if (idxCol.sortType != sortTypes[j]) {
                         // NULL FIRST for ascending and NULLS LAST
                         // for descending would actually match the default
                         ok = false;
@@ -476,42 +470,6 @@ public class Select extends Query {
         return null;
     }
 
-    /**
-     * Validates the cases where ORDER BY clause do not contains all indexed
-     * columns, but the related index path still would be valid for such search.
-     * Sometimes, the absence of a column in the ORDER BY clause does not alter
-     * the expected final result, and an index sorted scan could still be used.
-     * <pre>
-     * CREATE TABLE test(a, b);
-     * CREATE UNIQUE INDEX idx_test ON test(a, b);
-     * SELECT b FROM test WHERE a=22 AND b>10 order by b;
-     * </pre>
-     * More restrictive rule where one table query with indexed column not
-     * present in the ORDER BY clause is filtered with equality conditions (at
-     * least one) of type COLUMN = CONSTANT in a conjunctive fashion.
-     *
-     * @param sortColumn Column to be validated
-     * @return true if the column can be used implicitly, or false otherwise.
-     */
-    private boolean isSortColumnImplicit(TableFilter tableFilter,
-            Column sortColumn) {
-        if (filters.size() == 1 && condition != null
-                && !condition.isDisjunctive()) {
-            ArrayList<IndexCondition> conditions = tableFilter
-                    .getIndexConditionsForColumn(sortColumn);
-            if (conditions.isEmpty()) {
-                return false;
-            }
-            for (IndexCondition conditionExp : conditions) {
-                if (!conditionExp.isEquality(true)) {
-                    return false;
-                }
-            }
-            return true;
-        }
-        return false;
-    }
-
     private void queryDistinct(ResultTarget result, long limitRows) {
         // limitRows must be long, otherwise we get an int overflow
         // if limitRows is at or near Integer.MAX_VALUE

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

@@ -284,11 +284,6 @@ public class ConditionAndOr extends Condition {
         return left.getCost() + right.getCost();
     }
 
-    @Override
-    public boolean isDisjunctive() {
-        return andOrType == OR || left.isDisjunctive() || right.isDisjunctive();
-    }
-
     /**
      * Get the left or the right sub-expression of this condition.
      *

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

@@ -195,11 +195,6 @@ public class ConditionIn extends Condition {
         return cost;
     }
 
-    @Override
-    public boolean isDisjunctive() {
-        return true;
-    }
-
     /**
      * Add an additional element if possible. Example: given two conditions
      * A IN(1, 2) OR A=3, the constant 3 is added: A IN(1, 2, 3).

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

@@ -147,11 +147,6 @@ public class ConditionInConstantSet extends Condition {
         return cost;
     }
 
-    @Override
-    public boolean isDisjunctive() {
-        return true;
-    }
-
     /**
      * Add an additional element if possible. Example: given two conditions
      * A IN(1, 2) OR A=3, the constant 3 is added: A IN(1, 2, 3).

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

@@ -184,9 +184,4 @@ public class ConditionInSelect extends Condition {
         filter.addIndexCondition(IndexCondition.getInQuery(l, query));
     }
 
-    @Override
-    public boolean isDisjunctive() {
-        return true;
-    }
-
 }

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

@@ -262,15 +262,6 @@ public abstract class Expression {
         return this;
     }
 
-    /**
-     * Allows to check if the related expression is under conjunctive format.
-     *
-     * @return if the related expression has the logic operator OR.
-     */
-    public boolean isDisjunctive() {
-        return false;
-    }
-
     /**
      * Add conditions to a table filter if they can be evaluated.
      *

h2/src/main/org/h2/index/IndexCondition.java

@@ -334,22 +334,6 @@ public class IndexCondition {
         }
     }
 
-    /**
-     * Check if this index condition is of the type equality.
-     *
-     * @param constantExpression if the inner node is a constant expression
-     * @return true if this is a equality condition
-     */
-    public boolean isEquality(boolean constantExpression) {
-        switch (compareType) {
-        case Comparison.EQUAL:
-        case Comparison.EQUAL_NULL_SAFE:
-            return !constantExpression || expression.isConstant();
-        default:
-            return false;
-        }
-    }
-
     public int getCompareType() {
         return compareType;
     }

h2/src/main/org/h2/table/TableFilter.java

@@ -486,22 +486,6 @@ public class TableFilter implements ColumnResolver {
         indexConditions.add(condition);
     }
 
-    /**
-     * Return a list of index condition filtered by a specific column.
-     *
-     * @param column The column of the condition
-     * @return the filtered list
-     */
-    public ArrayList<IndexCondition> getIndexConditionsForColumn(Column column) {
-        ArrayList<IndexCondition> conditions = New.arrayList(indexConditions.size());
-        for (IndexCondition condition: indexConditions) {
-            if (column.equals(condition.getColumn())) {
-                conditions.add(condition);
-            }
-        }
-        return conditions;
-    }
-
     /**
      * Add a filter condition.
      *

h2/src/test/org/h2/test/db/TestIndex.java

@@ -39,6 +39,7 @@ public class TestIndex extends TestBase {
     @Override
     public void test() throws SQLException {
         deleteDb("index");
+        testOrderIndex();
         testIndexTypes();
         testHashIndexOnMemoryTable();
         testErrorMessage();
@@ -94,6 +95,23 @@ public class TestIndex extends TestBase {
         deleteDb("index");
     }
 
+    private void testOrderIndex() throws SQLException {
+        Connection conn = getConnection("index");
+        stat = conn.createStatement();
+        stat.execute("create table test(id int, name varchar)");
+        stat.execute("insert into test values (2, 'a'), (1, 'a')");
+        stat.execute("create index on test(name)");
+        ResultSet rs = stat.executeQuery(
+                "select id from test where name = 'a' order by id");
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertTrue(rs.next());
+        assertEquals(2, rs.getInt(1));
+        assertFalse(rs.next());
+        conn.close();
+        deleteDb("index");
+    }
+
     private void testIndexTypes() throws SQLException {
         Connection conn = getConnection("index");
         stat = conn.createStatement();

h2/src/test/org/h2/test/db/TestOptimizations.java

@@ -62,7 +62,7 @@ public class TestOptimizations extends TestBase {
         testNestedInSelect();
         testInSelectJoin();
         testMinMaxNullOptimization();
-        testUseIndexWhenAllColumnsNotInOrderBy();
+        // testUseIndexWhenAllColumnsNotInOrderBy();
         if (config.networked) {
             return;
         }