Issue 290: Conditions using subqueries were sometimes evaluated before much…

Issue 290: Conditions using subqueries were sometimes evaluated before much simpler conditions, which was very bad for performance. This includes subqueries returning a value, EXISTS subqueries, and IN(SELECT ..) subqueries.

h2/src/docsrc/html/changelog.html

@@ -18,7 +18,10 @@ Change Log
 <h1>Change Log</h1>
 
 <h2>Next Version (unreleased)</h2>
-<ul><li>Issue 288: Some queries with right outer joins or nested joins could throw a NullPointerException.
+<ul><li>Issue 290: Conditions using subqueries were sometimes evaluated before much simpler conditions,
+    which was very bad for performance. This includes subqueries returning a value, EXISTS subqueries,
+    and IN(SELECT ..) subqueries.
+</li><li>Issue 288: Some queries with right outer joins or nested joins could throw a NullPointerException.
 </li><li>The database can now be compiled with Java 7.
     Stub methods for the new JDBC 4.1 methods have been implemented,
     but the new features can't be used yet.

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

@@ -92,6 +92,19 @@ public abstract class Query extends Prepared {
      */
     public abstract double getCost();
 
+    /**
+     * Calculate the cost when used as a subquery.
+     * This method returns a value between 10 and 1000000,
+     * to ensure adding other values can't result in an integer overflow.
+     *
+     * @return the estimated cost as an integer
+     */
+    public int getCostAsExpression() {
+        // ensure the cost is not larger than 1 million,
+        // so that adding other values can't overflow
+        return (int) Math.min(1000000.0, 10.0 + 10.0 * getCost());
+    }
+
     /**
      * Get all tables that are involved in this query.
      *

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

@@ -1053,6 +1053,7 @@ public class Select extends Query {
                 buff.append("\n/* group sorted */");
             }
         }
+        // buff.append("\n/* cost: " + cost + " */");
         return buff.toString();
     }
 

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

@@ -11,6 +11,7 @@ import org.h2.engine.Session;
 import org.h2.result.ResultInterface;
 import org.h2.table.ColumnResolver;
 import org.h2.table.TableFilter;
+import org.h2.util.StringUtils;
 import org.h2.value.Value;
 import org.h2.value.ValueBoolean;
 
@@ -39,7 +40,7 @@ public class ConditionExists extends Condition {
     }
 
     public String getSQL() {
-        return "EXISTS(" + query.getPlanSQL() + ")";
+        return "EXISTS(\n" + StringUtils.indent(query.getPlanSQL(), 4, false) + ")";
     }
 
     public void updateAggregate(Session session) {
@@ -62,7 +63,7 @@ public class ConditionExists extends Condition {
     }
 
     public int getCost() {
-        return 10 + (int) (10 * query.getCost());
+        return query.getCostAsExpression();
     }
 
 }

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

@@ -146,7 +146,7 @@ public class ConditionInSelect extends Condition {
     }
 
     public int getCost() {
-        return left.getCost() + 10 + (int) (10 * query.getCost());
+        return left.getCost() + query.getCostAsExpression();
     }
 
     public void createIndexConditions(Session session, TableFilter filter) {

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

@@ -120,7 +120,7 @@ public class Subquery extends Expression {
     }
 
     public int getCost() {
-        return 10 + (int) (10 * query.getCost());
+        return query.getCostAsExpression();
     }
 
     public Expression[] getExpressionColumns(Session session) {

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

@@ -38,6 +38,7 @@ public class TestOptimizations extends TestBase {
 
     public void test() throws Exception {
         deleteDb("optimizations");
+        testExistsSubquery();
         testQueryCacheConcurrentUse();
         testQueryCacheResetParams();
         testRowId();
@@ -66,6 +67,18 @@ public class TestOptimizations extends TestBase {
         deleteDb("optimizations");
     }
 
+    private void testExistsSubquery() throws Exception {
+        Connection conn = getConnection("optimizations");
+        Statement stat = conn.createStatement();
+        stat.execute("create table test(id int) as select x from system_range(1, 10)");
+        ResultSet rs = stat.executeQuery("explain select * from test where exists(select 1 from test, test, test) and id = 10");
+        rs.next();
+        // ensure the ID = 10 part is evaluated first
+        assertContains(rs.getString(1), "WHERE (ID = 10)");
+        stat.execute("drop table test");
+        conn.close();
+    }
+
     private void testQueryCacheConcurrentUse() throws Exception {
         final Connection conn = getConnection("optimizations");
         Statement stat = conn.createStatement();