Merge pull request #700 from hendriks73/UseIndexDespiteNullsOrder

Ensured that an index is used for ordering, even if NULLS FIRST is sp…

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

@@ -5,10 +5,6 @@
  */
 package org.h2.command.dml;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
 import org.h2.api.ErrorCode;
 import org.h2.api.Trigger;
 import org.h2.command.CommandInterface;
@@ -16,40 +12,23 @@ import org.h2.engine.Constants;
 import org.h2.engine.Database;
 import org.h2.engine.Session;
 import org.h2.engine.SysProperties;
-import org.h2.expression.Alias;
-import org.h2.expression.Comparison;
-import org.h2.expression.ConditionAndOr;
-import org.h2.expression.Expression;
-import org.h2.expression.ExpressionColumn;
-import org.h2.expression.ExpressionVisitor;
-import org.h2.expression.Parameter;
+import org.h2.expression.*;
 import org.h2.index.Cursor;
 import org.h2.index.Index;
 import org.h2.index.IndexType;
 import org.h2.message.DbException;
-import org.h2.result.LazyResult;
-import org.h2.result.LocalResult;
-import org.h2.result.ResultInterface;
-import org.h2.result.ResultTarget;
-import org.h2.result.Row;
-import org.h2.result.SearchRow;
-import org.h2.result.SortOrder;
-import org.h2.table.Column;
-import org.h2.table.ColumnResolver;
-import org.h2.table.IndexColumn;
-import org.h2.table.JoinBatch;
-import org.h2.table.Table;
-import org.h2.table.TableFilter;
-import org.h2.table.TableView;
-import org.h2.util.ColumnNamer;
-import org.h2.util.New;
-import org.h2.util.StatementBuilder;
-import org.h2.util.StringUtils;
-import org.h2.util.ValueHashMap;
+import org.h2.result.*;
+import org.h2.table.*;
+import org.h2.util.*;
 import org.h2.value.Value;
 import org.h2.value.ValueArray;
 import org.h2.value.ValueNull;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+
 /**
  * This class represents a simple SELECT statement.
  *
@@ -409,13 +388,13 @@ public class Select extends Query {
             sortColumns.add(exprCol.getColumn());
         }
         Column[] sortCols = sortColumns.toArray(new Column[sortColumns.size()]);
-        int[] sortTypes = sort.getSortTypes();
         if (sortCols.length == 0) {
             // sort just on constants - can use scan index
             return topTableFilter.getTable().getScanIndex(session);
         }
         ArrayList<Index> list = topTableFilter.getTable().getIndexes();
         if (list != null) {
+            int[] sortTypes = sort.getSortTypesWithNullPosition();
             for (int i = 0, size = list.size(); i < size; i++) {
                 Index index = list.get(i);
                 if (index.getCreateSQL() == null) {
@@ -439,9 +418,7 @@ public class Select extends Query {
                         ok = false;
                         break;
                     }
-                    if (idxCol.sortType != sortTypes[j]) {
-                        // NULL FIRST for ascending and NULLS LAST
-                        // for descending would actually match the default
+                    if (SortOrder.addExplicitNullPosition(idxCol.sortType) != sortTypes[j]) {
                         ok = false;
                         break;
                     }

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

@@ -5,10 +5,6 @@
  */
 package org.h2.result;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
-
 import org.h2.command.dml.SelectOrderBy;
 import org.h2.engine.Database;
 import org.h2.engine.SysProperties;
@@ -22,6 +18,10 @@ import org.h2.util.Utils;
 import org.h2.value.Value;
 import org.h2.value.ValueNull;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+
 /**
  * A sort order represents an ORDER BY clause in a query.
  */
@@ -55,6 +55,16 @@ public class SortOrder implements Comparator<Value[]> {
     private static final int DEFAULT_NULL_SORT =
             SysProperties.SORT_NULLS_HIGH ? 1 : -1;
 
+    /**
+     * The default sort order bit for NULLs last.
+     */
+    private static final int DEFAULT_NULLS_LAST = SysProperties.SORT_NULLS_HIGH ? NULLS_LAST : NULLS_FIRST;
+
+    /**
+     * The default sort order bit for NULLs first.
+     */
+    private static final int DEFAULT_NULLS_FIRST = SysProperties.SORT_NULLS_HIGH ? NULLS_FIRST : NULLS_LAST;
+
     private final Database database;
 
     /**
@@ -261,4 +271,32 @@ public class SortOrder implements Comparator<Value[]> {
         return sortTypes;
     }
 
+    /**
+     * Returns sort order bit masks with {@link #NULLS_FIRST} or {@link #NULLS_LAST}
+     * explicitly set, depending on {@link SysProperties#SORT_NULLS_HIGH}.
+     *
+     * @return bit masks with either {@link #NULLS_FIRST} or {@link #NULLS_LAST} explicitly set.
+     */
+    public int[] getSortTypesWithNullPosition() {
+        final int[] sortTypes = this.sortTypes.clone();
+        for (int i=0, length = sortTypes.length; i<length; i++) {
+            sortTypes[i] = addExplicitNullPosition(sortTypes[i]);
+        }
+        return sortTypes;
+    }
+
+    /**
+     * Returns a sort type bit mask with {@link #NULLS_FIRST} or {@link #NULLS_LAST}
+     * explicitly set, depending on {@link SysProperties#SORT_NULLS_HIGH}.
+     *
+     * @param sortType sort type bit mask
+     * @return bit mask with either {@link #NULLS_FIRST} or {@link #NULLS_LAST} explicitly set.
+     */
+    public static int addExplicitNullPosition(final int sortType) {
+        if ((sortType & NULLS_FIRST) != NULLS_FIRST && (sortType & NULLS_LAST) != NULLS_LAST) {
+            return sortType | ((sortType & DESCENDING) == ASCENDING ? DEFAULT_NULLS_LAST : DEFAULT_NULLS_FIRST);
+        } else {
+            return sortType;
+        }
+    }
 }

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

@@ -5,18 +5,6 @@
  */
 package org.h2.test.db;
 
-import java.sql.Connection;
-import java.sql.PreparedStatement;
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.sql.Statement;
-import java.sql.Types;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.Random;
-import java.util.TreeSet;
-import java.util.concurrent.TimeUnit;
-
 import org.h2.api.ErrorCode;
 import org.h2.test.TestBase;
 import org.h2.tools.SimpleResultSet;
@@ -24,6 +12,13 @@ import org.h2.util.New;
 import org.h2.util.StringUtils;
 import org.h2.util.Task;
 
+import java.sql.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Random;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
 /**
  * Test various optimizations (query cache, optimization for MIN(..), and
  * MAX(..)).
@@ -81,6 +76,7 @@ public class TestOptimizations extends TestBase {
         testMinMaxCountOptimization(true);
         testMinMaxCountOptimization(false);
         testOrderedIndexes();
+        testIndexUseDespiteNullsFirst();
         testConvertOrToIn();
         deleteDb("optimizations");
     }
@@ -1034,6 +1030,85 @@ public class TestOptimizations extends TestBase {
         conn.close();
     }
 
+    private void testIndexUseDespiteNullsFirst() throws SQLException {
+        deleteDb("optimizations");
+        Connection conn = getConnection("optimizations");
+        Statement stat = conn.createStatement();
+
+        stat.execute("CREATE TABLE my_table(K1 INT)");
+        stat.execute("CREATE INDEX my_index ON my_table(K1)");
+        stat.execute("INSERT INTO my_table VALUES (NULL)");
+        stat.execute("INSERT INTO my_table VALUES (1)");
+        stat.execute("INSERT INTO my_table VALUES (2)");
+
+        ResultSet rs;
+        String result;
+
+
+        rs = stat.executeQuery(
+            "EXPLAIN PLAN FOR SELECT * FROM my_table " +
+                "ORDER BY K1 ASC NULLS FIRST");
+        rs.next();
+        result = rs.getString(1);
+        assertContains(result, "/* index sorted */");
+
+        rs = stat.executeQuery(
+            "SELECT * FROM my_table " +
+                "ORDER BY K1 ASC NULLS FIRST");
+        rs.next();
+        assertNull(rs.getObject(1));
+        rs.next();
+        assertEquals(1, rs.getInt(1));
+        rs.next();
+        assertEquals(2, rs.getInt(1));
+
+        // ===
+        rs = stat.executeQuery(
+            "EXPLAIN PLAN FOR SELECT * FROM my_table " +
+                "ORDER BY K1 DESC NULLS FIRST");
+        rs.next();
+        result = rs.getString(1);
+        if (result.contains("/* index sorted */")) {
+            fail(result + " does not contain: /* index sorted */");
+        }
+
+        rs = stat.executeQuery(
+            "SELECT * FROM my_table " +
+                "ORDER BY K1 DESC NULLS FIRST");
+        rs.next();
+        assertNull(rs.getObject(1));
+        rs.next();
+        assertEquals(2, rs.getInt(1));
+        rs.next();
+        assertEquals(1, rs.getInt(1));
+
+        // ===
+        rs = stat.executeQuery(
+            "EXPLAIN PLAN FOR SELECT * FROM my_table " +
+                "ORDER BY K1 ASC NULLS LAST");
+        rs.next();
+        result = rs.getString(1);
+        if (result.contains("/* index sorted */")) {
+            fail(result + " does not contain: /* index sorted */");
+        }
+
+        rs = stat.executeQuery(
+            "SELECT * FROM my_table " +
+                "ORDER BY K1 ASC NULLS LAST");
+        rs.next();
+        assertEquals(1, rs.getInt(1));
+        rs.next();
+        assertEquals(2, rs.getInt(1));
+        rs.next();
+        assertNull(rs.getObject(1));
+
+        // TODO: Test "EXPLAIN PLAN FOR SELECT * FROM my_table ORDER BY K1 DESC NULLS FIRST"
+        // Currently fails, as using the index when sorting DESC is currently not supported.
+
+        stat.execute("DROP TABLE my_table");
+        conn.close();
+    }
+
     private void testConvertOrToIn() throws SQLException {
         deleteDb("optimizations");
         Connection conn = getConnection("optimizations");