Issue 304: The condition [NOT] IN (SELECT ...) could throw the exception…

Issue 304: The condition [NOT] IN (SELECT ...) could throw the exception "Unexpected code path" if the subquery contained ORDER BY.

h2/src/docsrc/html/changelog.html

@@ -18,7 +18,8 @@ Change Log
 <h1>Change Log</h1>
 
 <h2>Next Version (unreleased)</h2>
-<ul><li>ALTER TABLE ALTER ADD / REMOVE /ALTER COLUMN dropped some dependent objects 
+<ul><li>Issue 304: The condition [NOT] IN (SELECT ...) could throw the exception "Unexpected code path" if the subquery contained ORDER BY.
+</li><li>ALTER TABLE ALTER ADD / REMOVE /ALTER COLUMN dropped some dependent objects 
     (access rights, triggers) of views that depend on the modified table.
 </li><li>CREATE OR REPLACE VIEW dropped some dependent objects (access rights, triggers)
     if the view already existed before.

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

@@ -51,6 +51,16 @@ public abstract class Query extends Prepared {
      */
     protected int sampleSize;
 
+    /**
+     * Whether the result must only contain distinct rows.
+     */
+    protected boolean distinct;
+
+    /**
+     * Whether the result needs to support random access.
+     */
+    protected boolean randomAccessResult;
+
     private int lastLimit;
     private long lastEvaluated;
     private LocalResult lastResult;
@@ -161,13 +171,6 @@ public abstract class Query extends Prepared {
      */
     public abstract void addGlobalCondition(Parameter param, int columnId, int comparisonType);
 
-    /**
-     * Set the distinct flag.
-     *
-     * @param b the new value
-     */
-    public abstract void setDistinct(boolean b);
-
     /**
      * Check if this expression and all sub-expressions can fulfill a criteria.
      * If any part returns false, the result is false.
@@ -189,6 +192,24 @@ public abstract class Query extends Prepared {
      */
     public abstract void fireBeforeSelectTriggers();
 
+    /**
+     * Set the distinct flag.
+     *
+     * @param b the new value
+     */
+    public void setDistinct(boolean b) {
+        distinct = b;
+    }
+
+    /**
+     * Whether results need to support random access.
+     *
+     * @param b the new value
+     */
+    public void setRandomAccessResult(boolean b) {
+        randomAccessResult = b;
+    }
+
     public boolean isQuery() {
         return true;
     }

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

@@ -72,7 +72,6 @@ public class Select extends Query {
     private ArrayList<Expression> group;
     private int[] groupIndex;
     private boolean[] groupByExpression;
-    private boolean distinct;
     private HashMap<Expression, Object> currentGroup;
     private int havingIndex;
     private boolean isGroupQuery, isGroupSortedQuery;
@@ -575,6 +574,10 @@ public class Select extends Query {
             result = createLocalResult(result);
             result.setDistinct();
         }
+        if (randomAccessResult) {
+            result = createLocalResult(result);
+            result.setRandomAccess();
+        }
         if (isGroupQuery && !isGroupSortedQuery) {
             result = createLocalResult(result);
         }
@@ -1068,10 +1071,6 @@ public class Select extends Query {
         return buff.toString();
     }
 
-    public void setDistinct(boolean b) {
-        distinct = b;
-    }
-
     public void setHaving(Expression having) {
         this.having = having;
     }

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

@@ -63,7 +63,6 @@ public class SelectUnion extends Query {
     private Expression[] expressionArray;
     private ArrayList<SelectOrderBy> orderList;
     private SortOrder sort;
-    private boolean distinct;
     private boolean isPrepared, checkInit;
     private boolean isForUpdate;
 
@@ -157,6 +156,9 @@ public class SelectUnion extends Query {
             right.setDistinct(true);
             result.setDistinct();
         }
+        if (randomAccessResult) {
+            result.setRandomAccess();
+        }
         switch (unionType) {
         case UNION:
         case EXCEPT:
@@ -200,6 +202,7 @@ public class SelectUnion extends Query {
         case INTERSECT: {
             LocalResult temp = new LocalResult(session, expressionArray, columnCount);
             temp.setDistinct();
+            temp.setRandomAccess();
             while (l.next()) {
                 temp.addRow(convert(l.currentRow(), columnCount));
             }
@@ -301,10 +304,6 @@ public class SelectUnion extends Query {
         return set;
     }
 
-    public void setDistinct(boolean b) {
-        distinct = b;
-    }
-
     public ArrayList<Expression> getExpressions() {
         return expressions;
     }

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

@@ -119,6 +119,7 @@ public class ConditionInSelect extends Condition {
     public Expression optimize(Session session) {
         left = left.optimize(session);
         query.setDistinct(true);
+        query.setRandomAccessResult(true);
         query.prepare();
         if (query.getColumnCount() != 1) {
             throw DbException.get(ErrorCode.SUBQUERY_IS_NOT_SINGLE_COLUMN);

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

@@ -41,6 +41,7 @@ public class LocalResult implements ResultInterface, ResultTarget {
     private ResultExternal external;
     private int diskOffset;
     private boolean distinct;
+    private boolean randomAccess;
     private boolean closed;
 
     /**
@@ -127,6 +128,7 @@ public class LocalResult implements ResultInterface, ResultTarget {
         copy.sort = this.sort;
         copy.distinctRows = this.distinctRows;
         copy.distinct = distinct;
+        copy.randomAccess = randomAccess;
         copy.currentRow = null;
         copy.offset = 0;
         copy.limit = -1;
@@ -152,6 +154,13 @@ public class LocalResult implements ResultInterface, ResultTarget {
         distinctRows = ValueHashMap.newInstance();
     }
 
+    /**
+     * Random access is required (containsDistinct).
+     */
+    public void setRandomAccess() {
+        this.randomAccess = true;
+    }
+
     /**
      * Remove the row from the result set if it exists.
      *
@@ -249,7 +258,11 @@ public class LocalResult implements ResultInterface, ResultTarget {
         rowCount++;
         if (rows.size() > maxMemoryRows && session.getDatabase().isPersistent()) {
             if (external == null) {
-                external = new ResultDiskBuffer(session, sort, values.length);
+                if (randomAccess) {
+                    external = new ResultTempTable(session, sort);
+                } else {
+                    external = new ResultDiskBuffer(session, sort, values.length);
+                }
             }
             addRowsToDisk();
         }
@@ -285,7 +298,11 @@ public class LocalResult implements ResultInterface, ResultTarget {
                             break;
                         }
                         if (external == null) {
-                            external = new ResultDiskBuffer(session, sort, list.length);
+                            if (randomAccess) {
+                                external = new ResultTempTable(session, sort);
+                            } else {
+                                external = new ResultDiskBuffer(session, sort, list.length);
+                            }
                         }
                         rows.add(list);
                         if (rows.size() > maxMemoryRows) {

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

@@ -544,16 +544,14 @@ public abstract class Table extends SchemaObjectBase {
         return new SimpleRow(new Value[columns.length]);
     }
 
-    Row getNullRow() {
-        synchronized (this) {
-            if (nullRow == null) {
-                nullRow = new Row(new Value[columns.length], 1);
-                for (int i = 0; i < columns.length; i++) {
-                    nullRow.setValue(i, ValueNull.INSTANCE);
-                }
+    synchronized Row getNullRow() {
+        if (nullRow == null) {
+            nullRow = new Row(new Value[columns.length], 1);
+            for (int i = 0; i < columns.length; i++) {
+                nullRow.setValue(i, ValueNull.INSTANCE);
             }
-            return nullRow;
         }
+        return nullRow;
     }
 
     public Column[] getColumns() {

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

@@ -36,6 +36,7 @@ public class TestCases extends TestBase {
     }
 
     public void test() throws Exception {
+        testMaxMemoryRowsDistinct();
         testDeleteTop();
         testUnicode();
         testOuterJoin();
@@ -88,6 +89,18 @@ public class TestCases extends TestBase {
         deleteDb("cases");
     }
 
+    private void testMaxMemoryRowsDistinct() throws SQLException {
+        deleteDb("cases");
+        Connection conn = getConnection("cases;max_memory_rows_distinct=1");
+        Statement stat = conn.createStatement();
+        stat.execute("create table test(id int primary key)");
+        stat.execute("insert into test values(1), (2)");
+        stat.execute("select * from dual where x not in (select id from test order by id)");
+        stat.execute("select * from dual where x not in (select id from test union select id from test)");
+        stat.execute("(select id from test order by id) intersect (select id from test order by id)");
+        conn.close();
+    }
+
     private void testUnicode() throws SQLException {
         deleteDb("cases");
         Connection conn = getConnection("cases");