OptimizerHints dropped in favor of SET FORCE_JOIN_ORDER setting, fixed join…

OptimizerHints dropped in favor of SET FORCE_JOIN_ORDER setting, fixed join order in case of TableFilter reorderings by Parser, added more tests.

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

@@ -13,6 +13,8 @@ import java.math.BigInteger;
 import java.nio.charset.Charset;
 import java.text.Collator;
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashSet;
 import org.h2.api.ErrorCode;
 import org.h2.api.Trigger;
@@ -177,6 +179,14 @@ public class Parser {
             CURRENT_TIME = 23, ROWNUM = 24;
     private static final int SPATIAL_INTERSECTS = 25;
 
+    private static final Comparator<TableFilter> TABLE_FILTER_COMPARATOR =
+            new Comparator<TableFilter>() {
+        @Override
+        public int compare(TableFilter o1, TableFilter o2) {
+            return o1 == o2 ? 0 : compareTableFilters(o1, o2);
+        }
+    };
+
     private final Database database;
     private final Session session;
     /**
@@ -208,6 +218,7 @@ public class Parser {
     private boolean rightsChecked;
     private boolean recompileAlways;
     private ArrayList<Parameter> indexedParameterList;
+    private int orderInFrom;
 
     public Parser(Session session) {
         this.database = session.getDatabase();
@@ -704,7 +715,7 @@ public class Parser {
         Update command = new Update(session);
         currentPrepared = command;
         int start = lastParseIndex;
-        TableFilter filter = readSimpleTableFilter();
+        TableFilter filter = readSimpleTableFilter(0);
         command.setTableFilter(filter);
         read("SET");
         if (readIf("(")) {
@@ -760,7 +771,7 @@ public class Parser {
         return command;
     }
 
-    private TableFilter readSimpleTableFilter() {
+    private TableFilter readSimpleTableFilter(int orderInFrom) {
         Table table = readTableOrView();
         String alias = null;
         if (readIf("AS")) {
@@ -772,7 +783,7 @@ public class Parser {
             }
         }
         return new TableFilter(session, table, alias, rightsChecked,
-                currentSelect);
+                currentSelect, orderInFrom);
     }
 
     private Delete parseDelete() {
@@ -784,7 +795,7 @@ public class Parser {
         currentPrepared = command;
         int start = lastParseIndex;
         readIf("FROM");
-        TableFilter filter = readSimpleTableFilter();
+        TableFilter filter = readSimpleTableFilter(0);
         command.setTableFilter(filter);
         if (readIf("WHERE")) {
             Expression condition = readExpression();
@@ -1186,7 +1197,7 @@ public class Parser {
                 return top;
             }
         } else if (readIf("VALUES")) {
-            table = parseValuesTable().getTable();
+            table = parseValuesTable(0).getTable();
         } else {
             String tableName = readIdentifierWithSchema(null);
             Schema schema = getSchema();
@@ -1236,7 +1247,7 @@ public class Parser {
         }
         alias = readFromAlias(alias);
         return new TableFilter(session, table, alias, rightsChecked,
-                currentSelect);
+                currentSelect, orderInFrom++);
     }
 
     private String readFromAlias(String alias) {
@@ -1610,7 +1621,7 @@ public class Parser {
     private TableFilter getNested(TableFilter n) {
         String joinTable = Constants.PREFIX_JOIN + parseIndex;
         TableFilter top = new TableFilter(session, getDualTable(true),
-                joinTable, rightsChecked, currentSelect);
+                joinTable, rightsChecked, currentSelect, n.getOrderInFrom());
         top.addJoin(n, false, true, null);
         return top;
     }
@@ -1873,6 +1884,38 @@ public class Parser {
             TableFilter filter = readTableFilter(false);
             parseJoinTableFilter(filter, command);
         } while (readIf(","));
+
+        // Parser can reorder joined table filters, need to explicitly sort them to
+        // get the order as it was in the original query.
+        if (session.isForceJoinOrder()) {
+            sortTableFilters(command.getTopFilters());
+        }
+    }
+
+    private static void sortTableFilters(ArrayList<TableFilter> filters) {
+        if (filters.size() < 2) {
+            return;
+        }
+        // Most probably we are already sorted correctly.
+        boolean sorted = true;
+        TableFilter prev = filters.get(0);
+        for (int i = 1; i < filters.size(); i++) {
+            TableFilter next = filters.get(i);
+            if (compareTableFilters(prev, next) > 0) {
+                sorted = false;
+                break;
+            }
+            prev = next;
+        }
+        // If not, then sort manually.
+        if (!sorted) {
+            Collections.sort(filters, TABLE_FILTER_COMPARATOR);
+        }
+    }
+
+    private static int compareTableFilters(TableFilter o1, TableFilter o2) {
+        assert o1.getOrderInFrom() != o2.getOrderInFrom();
+        return o1.getOrderInFrom() > o2.getOrderInFrom() ? 1 : -1;
     }
 
     private void parseJoinTableFilter(TableFilter top, final Select command) {
@@ -1976,7 +2019,7 @@ public class Parser {
                 // SYSTEM_RANGE(1,1)
                 Table dual = getDualTable(false);
                 TableFilter filter = new TableFilter(session, dual, null,
-                        rightsChecked, currentSelect);
+                        rightsChecked, currentSelect, 0);
                 command.addTableFilter(filter, true);
             } else {
                 parseSelectSimpleFromPart(command);
@@ -4312,7 +4355,7 @@ public class Parser {
     private Select parseValues() {
         Select command = new Select(session);
         currentSelect = command;
-        TableFilter filter = parseValuesTable();
+        TableFilter filter = parseValuesTable(0);
         ArrayList<Expression> list = New.arrayList();
         list.add(new Wildcard(null, null));
         command.setExpressions(list);
@@ -4321,7 +4364,7 @@ public class Parser {
         return command;
     }
 
-    private TableFilter parseValuesTable() {
+    private TableFilter parseValuesTable(int orderInFrom) {
         Schema mainSchema = database.getSchema(Constants.SCHEMA_MAIN);
         TableFunction tf = (TableFunction) Function.getFunction(database,
                 "TABLE");
@@ -4397,7 +4440,7 @@ public class Parser {
         tf.doneWithParameters();
         Table table = new FunctionTable(mainSchema, session, tf, tf);
         TableFilter filter = new TableFilter(session, table, null,
-                rightsChecked, currentSelect);
+                rightsChecked, currentSelect, orderInFrom);
         return filter;
     }
 

h2/src/main/org/h2/command/ddl/AlterTableAddConstraint.java

@@ -175,7 +175,7 @@ public class AlterTableAddConstraint extends SchemaCommand {
             int id = getObjectId();
             String name = generateConstraintName(table);
             ConstraintCheck check = new ConstraintCheck(getSchema(), id, name, table);
-            TableFilter filter = new TableFilter(session, table, null, false, null);
+            TableFilter filter = new TableFilter(session, table, null, false, null, 0);
             checkExpression.mapColumns(filter, 0);
             checkExpression = checkExpression.optimize(session);
             check.setExpression(checkExpression);

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

@@ -53,16 +53,6 @@ class Optimizer {
         this.session = session;
     }
 
-    /**
-     * Whether join reordering is enabled (it can be disabled by hint).
-     *
-     * @return {@code true} if yes
-     */
-    private static boolean isJoinReorderingEnabled() {
-        OptimizerHints hints = OptimizerHints.get();
-        return hints == null || hints.getJoinReorderEnabled();
-    }
-
     /**
      * How many filter to calculate using brute force. The remaining filters are
      * selected using a greedy algorithm which has a runtime of (1 + 2 + ... +
@@ -85,7 +75,7 @@ class Optimizer {
 
     private void calculateBestPlan() {
         cost = -1;
-        if (filters.length == 1 || !isJoinReorderingEnabled()) {
+        if (filters.length == 1 || session.isForceJoinOrder()) {
             testPlan(filters);
         } else {
             start = System.currentTimeMillis();
@@ -242,7 +232,7 @@ class Optimizer {
     /**
      * Calculate the best query plan to use.
      *
-     * @param parse If we do not need to really get the best plan because it is view a parsing stage.
+     * @param parse If we do not need to really get the best plan because it is a view parsing stage.
      */
     void optimize(boolean parse) {
         if (parse) {

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

-/*
- * Copyright 2004-2014 H2 Group. Multiple-Licensed under the MPL 2.0,
- * and the EPL 1.0 (http://h2database.com/html/license.html).
- * Initial Developer: H2 Group
- */
-package org.h2.command.dml;
-
-/**
- * Thread local hints for H2 query optimizer. All the ongoing queries in the
- * current thread will run with respect to these hints, so if they are needed
- * only for a single operation it is preferable to setup and drop them in
- * try-finally block.
- *
- * Currently works only in embedded mode.
- *
- * @author Sergi Vladykin
- */
-public class OptimizerHints {
-
-    private static final ThreadLocal<OptimizerHints> HINTS =
-            new ThreadLocal<OptimizerHints>();
-
-    private boolean joinReorderEnabled = true;
-
-    /**
-     * Set thread local hints or {@code null} to drop any existing hints.
-     *
-     * @param hints the hints
-     */
-    public static void set(OptimizerHints hints) {
-        if (hints != null) {
-            HINTS.set(hints);
-        } else {
-            HINTS.remove();
-        }
-    }
-
-    /**
-     * Get the current thread local hints or {@code null} if none.
-     *
-     * @return the hints
-     */
-    public static OptimizerHints get() {
-        return HINTS.get();
-    }
-
-    /**
-     * Set whether reordering of tables (or anything else in the {@code FROM}
-     * clause) is enabled. By default is {@code true}.
-     *
-     * @param joinReorderEnabled Flag value.
-     */
-    public void setJoinReorderEnabled(boolean joinReorderEnabled) {
-        this.joinReorderEnabled = joinReorderEnabled;
-    }
-
-    public boolean getJoinReorderEnabled() {
-        return joinReorderEnabled;
-    }
-}

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

@@ -506,6 +506,15 @@ public class Set extends Prepared {
             session.setJoinBatchEnabled(value == 1);
             break;
         }
+        case SetTypes.FORCE_JOIN_ORDER: {
+            int value = getIntValue();
+            if (value != 0 && value != 1) {
+                throw DbException.getInvalidValueException("FORCE_JOIN_ORDER",
+                        getIntValue());
+            }
+            session.setForceJoinOrder(value == 1);
+            break;
+        }
         default:
             DbException.throwInternalError("type="+type);
         }

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

@@ -233,6 +233,11 @@ public class SetTypes {
      */
     public static final int BATCH_JOINS = 44;
 
+    /**
+     * The type of SET FORCE_JOIN_ORDER statement.
+     */
+    public static final int FORCE_JOIN_ORDER = 45;
+
     private static final ArrayList<String> TYPES = New.arrayList();
 
     private SetTypes() {
@@ -286,6 +291,7 @@ public class SetTypes {
         list.add(QUERY_STATISTICS_MAX_ENTRIES, "QUERY_STATISTICS_MAX_ENTRIES");
         list.add(ROW_FACTORY, "ROW_FACTORY");
         list.add(BATCH_JOINS, "BATCH_JOINS");
+        list.add(FORCE_JOIN_ORDER, "FORCE_JOIN_ORDER");
     }
 
     /**

h2/src/main/org/h2/engine/Session.java

@@ -120,6 +120,7 @@ public class Session extends SessionWithState {
     private volatile SmallLRUCache<Object, ViewIndex> viewIndexCache;
     private HashMap<Object, ViewIndex> subQueryIndexCache;
     private boolean joinBatchEnabled;
+    private boolean forceJoinOrder;
 
     /**
      * Temporary LOBs from result sets. Those are kept for some time. The
@@ -153,6 +154,14 @@ public class Session extends SessionWithState {
         this.currentSchemaName = Constants.SCHEMA_MAIN;
     }
 
+    public void setForceJoinOrder(boolean forceJoinOrder) {
+        this.forceJoinOrder = forceJoinOrder;
+    }
+
+    public boolean isForceJoinOrder() {
+        return forceJoinOrder;
+    }
+
     public void setJoinBatchEnabled(boolean joinBatchEnabled) {
         this.joinBatchEnabled = joinBatchEnabled;
     }

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

@@ -399,7 +399,7 @@ public class Column {
      */
     public void prepareExpression(Session session) {
         if (defaultExpression != null) {
-            computeTableFilter = new TableFilter(session, table, null, false, null);
+            computeTableFilter = new TableFilter(session, table, null, false, null, 0);
             defaultExpression.mapColumns(computeTableFilter, 0);
             defaultExpression = defaultExpression.optimize(session);
         }

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

@@ -112,6 +112,7 @@ public class TableFilter implements ColumnResolver {
     private boolean foundOne;
     private Expression fullCondition;
     private final int hashCode;
+    private final int orderInFrom;
 
     /**
      * Create a new table filter object.
@@ -121,9 +122,10 @@ public class TableFilter implements ColumnResolver {
      * @param alias the alias name
      * @param rightsChecked true if rights are already checked
      * @param select the select statement
+     * @param orderInFrom Original order number of this table filter in FROM clause.
      */
     public TableFilter(Session session, Table table, String alias,
-            boolean rightsChecked, Select select) {
+            boolean rightsChecked, Select select, int orderInFrom) {
         this.session = session;
         this.table = table;
         this.alias = alias;
@@ -133,6 +135,11 @@ public class TableFilter implements ColumnResolver {
             session.getUser().checkRight(table, Right.SELECT);
         }
         hashCode = session.nextObjectId();
+        this.orderInFrom = orderInFrom;
+    }
+
+    public int getOrderInFrom() {
+        return orderInFrom;
     }
 
     public IndexCursor getIndexCursor() {

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

@@ -9,11 +9,12 @@ import java.sql.Connection;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
-import org.h2.command.dml.OptimizerHints;
+import java.util.Arrays;
 import org.h2.test.TestBase;
+import org.h2.util.StatementBuilder;
 
 /**
- * Test for optimizer hints.
+ * Test for optimizer hint SET FORCE_JOIN_ORDER.
  *
  * @author Sergi Vladykin
  */
@@ -30,55 +31,128 @@ public class TestOptimizerHints extends TestBase {
 
     @Override
     public void test() throws Exception {
-        if (config.networked) {
-            return;
-        }
         deleteDb("testOptimizerHints");
-        Connection conn = getConnection("testOptimizerHints");
+        Connection conn = getConnection("testOptimizerHints;FORCE_JOIN_ORDER=1");
         Statement s = conn.createStatement();
 
-        s.execute("create table t1(id int)");
-        s.execute("create table t2(id int, ref_id int)");
-
-        s.execute("insert into t1 values(1),(2),(3)");
-        s.execute("insert into t2 values(1,2),(2,3),(3,4),(4,6),(5,1),(6,4)");
-
-        s.execute("create unique index idx1_id on t1(id)");
-        s.execute("create index idx2_id on t2(id)");
-        s.execute("create index idx2_ref_id on t2(ref_id)");
-
-        enableJoinReordering(false);
-
-        try {
-            String plan;
-
-            plan = plan(s, "select * from t1, t2 where t1.id = t2.ref_id");
-            assertTrue(plan, plan.contains("INNER JOIN PUBLIC.T2"));
-
-            plan = plan(s, "select * from t2, t1 where t1.id = t2.ref_id");
-            assertTrue(plan, plan.contains("INNER JOIN PUBLIC.T1"));
-
-            plan = plan(s, "select * from t2, t1 where t1.id = 1");
-            assertTrue(plan, plan.contains("INNER JOIN PUBLIC.T1"));
-
-            plan = plan(s, "select * from t2, t1 where t1.id = t2.ref_id and t2.id = 1");
-            assertTrue(plan, plan.contains("INNER JOIN PUBLIC.T1"));
+        s.execute("create table t1(id int unique)");
+        s.execute("create table t2(id int unique, t1_id int)");
+        s.execute("create table t3(id int unique)");
+        s.execute("create table t4(id int unique, t2_id int, t3_id int)");
+
+        String plan;
+
+        plan = plan(s, "select * from t1, t2 where t1.id = t2.t1_id");
+        assertTrue(plan, plan.contains("INNER JOIN PUBLIC.T2"));
+
+        plan = plan(s, "select * from t2, t1 where t1.id = t2.t1_id");
+        assertTrue(plan, plan.contains("INNER JOIN PUBLIC.T1"));
+
+        plan = plan(s, "select * from t2, t1 where t1.id = 1");
+        assertTrue(plan, plan.contains("INNER JOIN PUBLIC.T1"));
+
+        plan = plan(s, "select * from t2, t1 where t1.id = t2.t1_id and t2.id = 1");
+        assertTrue(plan, plan.contains("INNER JOIN PUBLIC.T1"));
+
+        plan = plan(s, "select * from t1, t2 where t1.id = t2.t1_id and t2.id = 1");
+        assertTrue(plan, plan.contains("INNER JOIN PUBLIC.T2"));
+
+        checkPlanComma(s, "t1", "t2", "t3", "t4");
+        checkPlanComma(s, "t4", "t2", "t3", "t1");
+        checkPlanComma(s, "t2", "t1", "t3", "t4");
+        checkPlanComma(s, "t1", "t4", "t3", "t2");
+        checkPlanComma(s, "t2", "t1", "t4", "t3");
+        checkPlanComma(s, "t4", "t3", "t2", "t1");
+
+        boolean on = false;
+        boolean left = false;
+
+        checkPlanJoin(s, on, left, "t1", "t2", "t3", "t4");
+        checkPlanJoin(s, on, left, "t4", "t2", "t3", "t1");
+        checkPlanJoin(s, on, left, "t2", "t1", "t3", "t4");
+        checkPlanJoin(s, on, left, "t1", "t4", "t3", "t2");
+        checkPlanJoin(s, on, left, "t2", "t1", "t4", "t3");
+        checkPlanJoin(s, on, left, "t4", "t3", "t2", "t1");
+
+        on = false;
+        left = true;
+
+        checkPlanJoin(s, on, left, "t1", "t2", "t3", "t4");
+        checkPlanJoin(s, on, left, "t4", "t2", "t3", "t1");
+        checkPlanJoin(s, on, left, "t2", "t1", "t3", "t4");
+        checkPlanJoin(s, on, left, "t1", "t4", "t3", "t2");
+        checkPlanJoin(s, on, left, "t2", "t1", "t4", "t3");
+        checkPlanJoin(s, on, left, "t4", "t3", "t2", "t1");
+
+        on = true;
+        left = false;
+
+        checkPlanJoin(s, on, left, "t1", "t2", "t3", "t4");
+        checkPlanJoin(s, on, left, "t4", "t2", "t3", "t1");
+        checkPlanJoin(s, on, left, "t2", "t1", "t3", "t4");
+        checkPlanJoin(s, on, left, "t1", "t4", "t3", "t2");
+        checkPlanJoin(s, on, left, "t2", "t1", "t4", "t3");
+        checkPlanJoin(s, on, left, "t4", "t3", "t2", "t1");
+
+        on = true;
+        left = true;
+
+        checkPlanJoin(s, on, left, "t1", "t2", "t3", "t4");
+        checkPlanJoin(s, on, left, "t4", "t2", "t3", "t1");
+        checkPlanJoin(s, on, left, "t2", "t1", "t3", "t4");
+        checkPlanJoin(s, on, left, "t1", "t4", "t3", "t2");
+        checkPlanJoin(s, on, left, "t2", "t1", "t4", "t3");
+        checkPlanJoin(s, on, left, "t4", "t3", "t2", "t1");
+
+        s.close();
+        conn.close();
+        deleteDb("testOptimizerHints");
+    }
 
-            plan = plan(s, "select * from t1, t2 where t1.id = t2.ref_id and t2.id = 1");
-            assertTrue(plan, plan.contains("INNER JOIN PUBLIC.T2"));
-        } finally {
-            enableJoinReordering(true);
+    private void checkPlanComma(Statement s, String ... t) throws SQLException {
+        StatementBuilder from = new StatementBuilder();
+        for (String table : t) {
+            from.appendExceptFirst(", ");
+            from.append(table);
+        }
+        String plan = plan(s, "select 1 from " + from.toString() + " where t1.id = t2.t1_id "
+                + "and t2.id = t4.t2_id and t3.id = t4.t3_id");
+        int prev = plan.indexOf("FROM PUBLIC." + t[0].toUpperCase());
+        for (int i = 1; i < t.length; i++) {
+            int next = plan.indexOf("INNER JOIN PUBLIC." + t[i].toUpperCase());
+            assertTrue("Wrong plan for : " + Arrays.toString(t) + "\n" + plan, next > prev);
+            prev = next;
         }
-        deleteDb("testOptimizerHints");
     }
 
-    /**
-     * @param enable Enabled.
-     */
-    private void enableJoinReordering(boolean enable) {
-        OptimizerHints hints = new OptimizerHints();
-        hints.setJoinReorderEnabled(enable);
-        OptimizerHints.set(hints);
+    private void checkPlanJoin(Statement s, boolean on, boolean left, String ... t) throws SQLException {
+        StatementBuilder from = new StatementBuilder();
+        for (int i = 0; i < t.length; i++) {
+            if (i != 0) {
+                if (left) {
+                    from.append(" left join ");
+                } else {
+                    from.append(" inner join ");
+                }
+            }
+            from.append(t[i]);
+            if (on && i != 0) {
+                from.append(" on 1=1 ");
+            }
+        }
+        String plan = plan(s, "select 1 from " + from.toString() + " where t1.id = t2.t1_id "
+                + "and t2.id = t4.t2_id and t3.id = t4.t3_id");
+        int prev = plan.indexOf("FROM PUBLIC." + t[0].toUpperCase());
+        for (int i = 1; i < t.length; i++) {
+            int next = plan.indexOf(
+                    (!left ? "INNER JOIN PUBLIC." : on ? "LEFT OUTER JOIN PUBLIC." : "PUBLIC.") + 
+                    t[i].toUpperCase());
+            if (prev > next) {
+                System.err.println(plan);
+                fail("Wrong plan for : " + Arrays.toString(t) + "\n" + plan);
+            }
+            prev = next;
+        }
     }
 
     /**

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

@@ -27,7 +27,6 @@ import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.h2.api.TableEngine;
 import org.h2.command.ddl.CreateTableData;
-import org.h2.command.dml.OptimizerHints;
 import org.h2.engine.Constants;
 import org.h2.engine.Session;
 import org.h2.expression.Expression;
@@ -410,10 +409,6 @@ public class TestTableEngines extends TestBase {
     }
     
     private void testBatchedJoin() throws SQLException {
-        if (config.networked) {
-            // networked test is disabled because it relies on OptimizerHints which is ThreadLocal
-            return;
-        }
         deleteDb("testBatchedJoin");
         Connection conn = getConnection("testBatchedJoin;OPTIMIZE_REUSE_RESULTS=0;BATCH_JOINS=1");
         Statement stat = conn.createStatement();
@@ -431,7 +426,7 @@ public class TestTableEngines extends TestBase {
             }
         });
         
-        enableJoinReordering(false);
+        forceJoinOrder(stat, true);
         try {
             doTestBatchedJoinSubQueryUnion(stat);
             
@@ -469,22 +464,14 @@ public class TestTableEngines extends TestBase {
             
             assertTrue(TreeSetIndex.lookupBatches.get() > 0);
         } finally {
-            enableJoinReordering(true);
+            forceJoinOrder(stat, false);
             TreeSetIndex.exec.shutdownNow();
         }
         deleteDb("testBatchedJoin");
     }
     
-    /**
-     * @param enable Enabled.
-     */
-    private void enableJoinReordering(boolean enable) {
-        OptimizerHints hints = null;
-        if (!enable) {
-            hints = new OptimizerHints();
-            hints.setJoinReorderEnabled(false);
-        }
-        OptimizerHints.set(hints);
+    private void forceJoinOrder(Statement s, boolean force) throws SQLException {
+        s.executeUpdate("SET FORCE_JOIN_ORDER " + force);
     }
 
     private void checkPlan(Statement stat, String sql) throws SQLException {