subquery optimiser now works with joins that have subqueries

core/src/main/java/com/questdb/ql/model/QueryModel.java

@@ -52,6 +52,8 @@ import com.questdb.ql.parser.QueryError;
 import com.questdb.std.*;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
+import java.util.ArrayDeque;
+
 public class QueryModel implements Mutable {
     public static final QueryModelFactory FACTORY = new QueryModelFactory();
     public static final int ORDER_DIRECTION_ASCENDING = 0;
@@ -75,6 +77,7 @@ public class QueryModel implements Mutable {
     // list of "and" concatenated expressions
     private final ObjList<ExprNode> parsedWhere = new ObjList<>();
     private final IntHashSet parsedWhereConsts = new IntHashSet();
+    private final ArrayDeque<ExprNode> exprNodeStack = new ArrayDeque<>();
     private CharSequenceObjHashMap<Parameter> parameterMap = new CharSequenceObjHashMap<>();
     private ExprNode whereClause;
     private ExprNode postJoinWhereClause;
@@ -95,6 +98,7 @@ public class QueryModel implements Mutable {
     private VirtualColumn limitLoVc;
     private VirtualColumn limitHiVc;
 
+
     private QueryModel() {
         joinModels.add(this);
     }
@@ -161,6 +165,7 @@ public class QueryModel implements Mutable {
         parameterMap.clear();
         timestamp = null;
         orderColumnIndices.clear();
+        exprNodeStack.clear();
     }
 
     public JournalMetadata collectJournalMetadata(JournalReaderFactory factory) throws ParserException, JournalException {
@@ -185,7 +190,15 @@ public class QueryModel implements Mutable {
 
     public void createColumnNameHistogram(JournalReaderFactory factory) throws JournalException, ParserException {
         columnNameHistogram.clear();
-        createColumnNameHistogram0(columnNameHistogram, getNestedModel(), factory, false);
+        createColumnNameHistogram0(columnNameHistogram, this, factory, false);
+    }
+
+    public void createColumnNameHistogram(RecordSource rs) {
+        columnNameHistogram.clear();
+        RecordMetadata m = rs.getMetadata();
+        for (int i = 0, n = m.getColumnCount(); i < n; i++) {
+            columnNameHistogram.increment(m.getColumnName(i));
+        }
     }
 
     public ExprNode getAlias() {
@@ -383,6 +396,34 @@ public class QueryModel implements Mutable {
         return ordered;
     }
 
+    /**
+     * Splits "where" clauses into "and" chunks
+     */
+    public ObjList<ExprNode> parseWhereClause() {
+        ExprNode n = getWhereClause();
+        // pre-order traversal
+        exprNodeStack.clear();
+        while (!exprNodeStack.isEmpty() || n != null) {
+            if (n != null) {
+                switch (n.token) {
+                    case "and":
+                        if (n.rhs != null) {
+                            exprNodeStack.push(n.rhs);
+                        }
+                        n = n.lhs;
+                        break;
+                    default:
+                        addParsedWhereNode(n);
+                        n = null;
+                        break;
+                }
+            } else {
+                n = exprNodeStack.poll();
+            }
+        }
+        return getParsedWhere();
+    }
+
     public CharSequence plan() {
         planSink.clear();
         plan(planSink, 0);
@@ -546,7 +587,6 @@ public class QueryModel implements Mutable {
         sink.put('\n');
     }
 
-
     public enum JoinType {
         INNER, OUTER, CROSS, ASOF
     }

core/src/main/java/com/questdb/ql/parser/LiteralMatcher.java

@@ -51,10 +51,6 @@ class LiteralMatcher implements PostOrderTreeTraversalAlgo.Visitor {
         this.algo = algo;
     }
 
-    public void of(String alias) {
-        this.alias = alias;
-    }
-
     @Override
     public void visit(ExprNode node) throws ParserException {
         if (node.type == ExprNode.NodeType.LITERAL && match) {
@@ -65,7 +61,7 @@ class LiteralMatcher implements PostOrderTreeTraversalAlgo.Visitor {
             }
 
             if (f > 0) {
-                throw QueryError.$(node.position, "Ambiguous column name");
+                throw QueryError.ambiguousColumn(node.position);
             }
 
             if (alias == null) {
@@ -82,7 +78,7 @@ class LiteralMatcher implements PostOrderTreeTraversalAlgo.Visitor {
 
             if (Chars.equals(columnName.alias(), alias) && (f = names.get(columnName.name())) > -1) {
                 if (f > 0) {
-                    throw QueryError.$(node.position, "Ambiguous column name");
+                    throw QueryError.ambiguousColumn(node.position);
                 }
                 node.token = columnName.name().toString();
                 return;
@@ -91,9 +87,10 @@ class LiteralMatcher implements PostOrderTreeTraversalAlgo.Visitor {
         }
     }
 
-    boolean matches(ExprNode node, CharSequenceIntHashMap names) throws ParserException {
+    boolean matches(ExprNode node, CharSequenceIntHashMap names, String alias) throws ParserException {
         this.match = true;
         this.names = names;
+        this.alias = alias;
         algo.traverse(node, this);
         return match;
     }

core/src/main/java/com/questdb/ql/parser/QueryCompiler.java

@@ -535,6 +535,8 @@ public class QueryCompiler {
     private RecordSource compile(QueryModel model, JournalReaderFactory factory) throws JournalException, ParserException {
         RecordSource rs;
 
+        optimiseSubQueries(model, factory);
+
         if (model.getJoinModels().size() > 1) {
             rs = compileJoins(model, factory);
         } else if (model.getJournalName() != null) {
@@ -570,7 +572,7 @@ public class QueryCompiler {
             }
 
             if (needColumnNameHistogram) {
-                createColumnNameHistogram(model, slave);
+                model.createColumnNameHistogram(slave);
             }
 
             // check if this is the root of joins
@@ -807,7 +809,7 @@ public class QueryCompiler {
     private RecordSource compileSubQuery(QueryModel model, JournalReaderFactory factory) throws JournalException, ParserException {
 
         applyLimit(model);
-        optimiseSubQueries(model, factory);
+//        optimiseSubQueries(model, factory);
 
         RecordSource rs = compile(model.getNestedModel(), factory);
         if (model.getWhereClause() == null) {
@@ -897,14 +899,6 @@ public class QueryCompiler {
         }
     }
 
-    private void createColumnNameHistogram(QueryModel model, RecordSource rs) {
-        CharSequenceIntHashMap map = model.getColumnNameHistogram();
-        RecordMetadata m = rs.getMetadata();
-        for (int i = 0, n = m.getColumnCount(); i < n; i++) {
-            map.increment(m.getColumnName(i));
-        }
-    }
-
     private RecordSource createHashJoin(QueryModel model, RecordSource master, RecordSource slave) throws ParserException {
         JoinContext jc = model.getContext();
         RecordMetadata bm = master.getMetadata();
@@ -1293,28 +1287,54 @@ public class QueryCompiler {
     }
 
     private void optimiseSubQueries(QueryModel model, JournalReaderFactory factory) throws JournalException, ParserException {
-        QueryModel m = model;
         QueryModel nm;
-        while ((nm = m.getNestedModel()) != null) {
-            m.createColumnNameHistogram(factory);
-            processAndConditions(m, m.getWhereClause());
-            literalMatcher.of(m.getAlias() != null ? m.getAlias().token : null);
-
-            ExprNode nmWhere = nm.getWhereClause();
-            ExprNode thisWhere = null;
-            ObjList<ExprNode> w = m.getParsedWhere();
-            for (int i = 0, n = w.size(); i < n; i++) {
-                ExprNode node = w.getQuick(i);
-                if (literalMatcher.matches(node, m.getColumnNameHistogram())) {
-                    nmWhere = concatFilters(nmWhere, node);
-                } else {
-                    thisWhere = concatFilters(thisWhere, node);
+        ObjList<QueryModel> jm = model.getJoinModels();
+        ObjList<ExprNode> where = model.parseWhereClause();
+        ExprNode thisWhere = null;
+
+        // create name histograms
+        for (int i = 0, n = jm.size(); i < n; i++) {
+            if ((nm = jm.getQuick(i).getNestedModel()) != null) {
+                nm.createColumnNameHistogram(factory);
+            }
+        }
+
+        // match each of where conditions to join models
+        // if "where" matches two models, we have ambiguous column name
+        for (int j = 0, k = where.size(); j < k; j++) {
+            ExprNode node = where.getQuick(j);
+            int matchModel = -1;
+            for (int i = 0, n = jm.size(); i < n; i++) {
+                QueryModel qm = jm.getQuick(i);
+                nm = qm.getNestedModel();
+                if (nm != null) {
+                    if (literalMatcher.matches(node, nm.getColumnNameHistogram(), qm.getAlias() != null ? qm.getAlias().token : null)) {
+                        if (matchModel > -1) {
+                            throw QueryError.ambiguousColumn(node.position);
+                        }
+                        matchModel = i;
+                    }
                 }
             }
 
-            nm.setWhereClause(nmWhere);
-            m.setWhereClause(thisWhere);
-            m = nm;
+            if (matchModel > -1) {
+                nm = jm.getQuick(matchModel).getNestedModel();
+                nm.setWhereClause(concatFilters(nm.getWhereClause(), node));
+            } else {
+                thisWhere = concatFilters(thisWhere, node);
+            }
+        }
+
+        model.getParsedWhere().clear();
+        model.setWhereClause(thisWhere);
+
+        // recursively apply same logic to nested model of each of join model
+        for (int i = 0, n = jm.size(); i < n; i++) {
+            QueryModel qm = jm.getQuick(i);
+            nm = qm.getNestedModel();
+            if (nm != null) {
+                optimiseSubQueries(nm, factory);
+            }
         }
     }
 
@@ -1355,35 +1375,6 @@ public class QueryCompiler {
         return model.plan();
     }
 
-    /**
-     * Splits "where" clauses into "and" chunks
-     *
-     * @param node expression n
-     */
-    private void processAndConditions(QueryModel parent, ExprNode node) {
-        ExprNode n = node;
-        // pre-order traversal
-        exprNodeStack.clear();
-        while (!exprNodeStack.isEmpty() || n != null) {
-            if (n != null) {
-                switch (n.token) {
-                    case "and":
-                        if (n.rhs != null) {
-                            exprNodeStack.push(n.rhs);
-                        }
-                        n = n.lhs;
-                        break;
-                    default:
-                        parent.addParsedWhereNode(n);
-                        n = null;
-                        break;
-                }
-            } else {
-                n = exprNodeStack.poll();
-            }
-        }
-    }
-
     private void processEmittedJoinClauses(QueryModel model) {
         // process emitted join conditions
         do {
@@ -1650,7 +1641,7 @@ public class QueryCompiler {
                 }
 
                 if (index > -1) {
-                    throw QueryError.$(position, "Ambiguous column name");
+                    throw QueryError.ambiguousColumn(position);
                 }
 
                 index = i;
@@ -1709,7 +1700,7 @@ public class QueryCompiler {
 
                 // check if this column is exists in more than one journal/result set
                 if (columnNameHistogram.get(node.token) > 0) {
-                    throw QueryError.$(node.position, "Ambiguous column name");
+                    throw QueryError.ambiguousColumn(node.position);
                 }
 
                 // add to selected columns

core/src/main/java/com/questdb/ql/parser/QueryError.java

@@ -50,6 +50,10 @@ public final class QueryError implements QueryErrorBuilder {
         return position(position).$(message).$();
     }
 
+    public static ParserException ambiguousColumn(int position) {
+        return position(position).$("Ambiguous column name").$();
+    }
+
     public static CharSequence getMessage() {
         return INSTANCE.tl.get().sink;
     }

core/src/main/java/com/questdb/ql/parser/VirtualColumnBuilder.java

 /*******************************************************************************
- * ___                  _   ____  ____
- * / _ \ _   _  ___  ___| |_|  _ \| __ )
- * | | | | | | |/ _ \/ __| __| | | |  _ \
- * | |_| | |_| |  __/\__ \ |_| |_| | |_) |
- * \__\_\\__,_|\___||___/\__|____/|____/
- * <p>
+ *    ___                  _   ____  ____
+ *   / _ \ _   _  ___  ___| |_|  _ \| __ )
+ *  | | | | | | |/ _ \/ __| __| | | |  _ \
+ *  | |_| | |_| |  __/\__ \ |_| |_| | |_) |
+ *   \__\_\\__,_|\___||___/\__|____/|____/
+ *
  * Copyright (C) 2014-2016 Appsicle
- * <p>
+ *
  * This program is free software: you can redistribute it and/or  modify
  * it under the terms of the GNU Affero General Public License, version 3,
  * as published by the Free Software Foundation.
- * <p>
+ *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU Affero General Public License for more details.
- * <p>
+ *
  * You should have received a copy of the GNU Affero General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- * <p>
+ *
  * As a special exception, the copyright holders give permission to link the
  * code of portions of this program with the OpenSSL library under certain
  * conditions as described in each individual source file and distribute
@@ -30,6 +30,7 @@
  * delete this exception statement from your version. If you delete this
  * exception statement from all source files in the program, then also delete
  * it in the license file.
+ *
  ******************************************************************************/
 
 package com.questdb.ql.parser;
@@ -118,7 +119,7 @@ class VirtualColumnBuilder implements PostOrderTreeTraversalAlgo.Visitor {
     private VirtualColumn lookupColumn(ExprNode node) throws ParserException {
         try {
             if (columnNameHistogram.get(node.token) > 0) {
-                throw QueryError.$(node.position, "Ambiguous column name");
+                throw QueryError.ambiguousColumn(node.position);
             }
             int index = metadata.getColumnIndex(node.token);
             switch (metadata.getColumnQuick(index).getType()) {

core/src/test/java/com/questdb/ql/SubqueryOptimiserTest.java

@@ -76,6 +76,31 @@ public class SubqueryOptimiserTest extends AbstractOptimiserTest {
         sink.clear();
     }
 
+    @Test
+    public void testAmbiguousColumn() throws Exception {
+        try {
+            compiler.compileSource(factory, "(((tab order by y) where y = 5) a join tex b on a.id = b.id) a where a.x = 10 and id > 100");
+        } catch (ParserException e) {
+            TestUtils.assertEquals("Ambiguous column name", QueryError.getMessage());
+        }
+    }
+
+    @Test
+    public void testJoinRecursiveJoinSubQueries() throws Exception {
+        RecordSource rs = compiler.compileSource(factory, "(((tab order by y) where y = 5) a join tex b on a.id = b.id) a where a.x = 10 and a.amount > 100");
+        sink.put(rs);
+        TestUtils.assertEquals("{\"op\":\"HashJoinRecordSource\",\"master\":{\"op\":\"RBTreeSortedRecordSource\",\"byRowId\":true,\"src\":{\"op\":\"JournalSource\",\"psrc\":{\"op\":\"JournalPartitionSource\",\"journal\":\"tab\"},\"rsrc\":{\"op\":\"FilteredRowSource\",\"rsrc\":{\"op\":\"AllRowSource\"}}}},\"slave\":{\"op\":\"JournalSource\",\"psrc\":{\"op\":\"JournalPartitionSource\",\"journal\":\"tex\"},\"rsrc\":{\"op\":\"FilteredRowSource\",\"rsrc\":{\"op\":\"AllRowSource\"}}},\"joinOn\":[[\"id\"],[\"id\"]]}",
+                sink);
+    }
+
+    @Test
+    public void testJoinSubQueries() throws Exception {
+        RecordSource rs = compiler.compileSource(factory, "((tab order by y) a join tex b on a.id = b.id) a where a.x = 10 and a.amount > 100");
+        sink.put(rs);
+        TestUtils.assertEquals("{\"op\":\"HashJoinRecordSource\",\"master\":{\"op\":\"RBTreeSortedRecordSource\",\"byRowId\":true,\"src\":{\"op\":\"JournalSource\",\"psrc\":{\"op\":\"JournalPartitionSource\",\"journal\":\"tab\"},\"rsrc\":{\"op\":\"FilteredRowSource\",\"rsrc\":{\"op\":\"AllRowSource\"}}}},\"slave\":{\"op\":\"JournalSource\",\"psrc\":{\"op\":\"JournalPartitionSource\",\"journal\":\"tex\"},\"rsrc\":{\"op\":\"FilteredRowSource\",\"rsrc\":{\"op\":\"AllRowSource\"}}},\"joinOn\":[[\"id\"],[\"id\"]]}",
+                sink);
+    }
+
     @Test
     public void testJoinSubQueryFilter() throws Exception {
         sink.put(compiler.compileSource(factory, "(tab a join tex b on a.id = b.id) a where a.x = 10"));
@@ -85,14 +110,9 @@ public class SubqueryOptimiserTest extends AbstractOptimiserTest {
 
     @Test
     public void testJoinSubQueryFilter2() throws Exception {
-        try {
-            sink.put(compiler.compileSource(factory, "(tab a join tex b on a.id = b.id) a where a.amount = 10"));
-            TestUtils.assertEquals("{\"op\":\"HashJoinRecordSource\",\"master\":{\"op\":\"JournalSource\",\"psrc\":{\"op\":\"JournalPartitionSource\",\"journal\":\"tab\"},\"rsrc\":{\"op\":\"AllRowSource\"}},\"slave\":{\"op\":\"JournalSource\",\"psrc\":{\"op\":\"JournalPartitionSource\",\"journal\":\"tex\"},\"rsrc\":{\"op\":\"FilteredRowSource\",\"rsrc\":{\"op\":\"AllRowSource\"}}},\"joinOn\":[[\"id\"],[\"id\"]]}",
-                    sink);
-        } catch (ParserException e) {
-            e.printStackTrace();
-            System.out.println(QueryError.getMessage());
-        }
+        sink.put(compiler.compileSource(factory, "(tab a join tex b on a.id = b.id) a where a.amount = 10"));
+        TestUtils.assertEquals("{\"op\":\"HashJoinRecordSource\",\"master\":{\"op\":\"JournalSource\",\"psrc\":{\"op\":\"JournalPartitionSource\",\"journal\":\"tab\"},\"rsrc\":{\"op\":\"AllRowSource\"}},\"slave\":{\"op\":\"JournalSource\",\"psrc\":{\"op\":\"JournalPartitionSource\",\"journal\":\"tex\"},\"rsrc\":{\"op\":\"FilteredRowSource\",\"rsrc\":{\"op\":\"AllRowSource\"}}},\"joinOn\":[[\"id\"],[\"id\"]]}",
+                sink);
     }
 
     @Test
@@ -144,4 +164,5 @@ public class SubqueryOptimiserTest extends AbstractOptimiserTest {
         TestUtils.assertEquals("{\"op\":\"FilteredJournalRecordSource\",\"src\":{\"op\":\"SelectedColumnsRecordSource\",\"src\":{\"op\":\"VirtualColumnRecordSource\",\"src\":{\"op\":\"JournalSource\",\"psrc\":{\"op\":\"JournalPartitionSource\",\"journal\":\"tab\"},\"rsrc\":{\"op\":\"AllRowSource\"}}}},\"filter\":\"a.k = 10\"}",
                 sink);
     }
+
 }

core/src/test/java/com/questdb/ql/parser/JoinQueryTest.java

@@ -172,7 +172,10 @@ public class JoinQueryTest extends AbstractOptimiserTest {
                         "                \"journal\": \"employees\"\n" +
                         "              },\n" +
                         "              \"rsrc\": {\n" +
-                        "                \"op\": \"AllRowSource\"\n" +
+                        "                \"op\": \"FilteredRowSource\",\n" +
+                        "                \"rsrc\": {\n" +
+                        "                  \"op\": \"AllRowSource\"\n" +
+                        "                }\n" +
                         "              }\n" +
                         "            }\n" +
                         "          }\n" +
@@ -181,7 +184,7 @@ public class JoinQueryTest extends AbstractOptimiserTest {
                         "      \"masterTsIndex\": 7,\n" +
                         "      \"slaveTsIndex\": 3\n" +
                         "    },\n" +
-                        "    \"filter\": \"e.lastName \\u003d \\u0027x\\u0027 and e.blah \\u003d \\u0027y\\u0027\"\n" +
+                        "    \"filter\": \"e.blah \\u003d \\u0027y\\u0027\"\n" +
                         "  },\n" +
                         "  \"slave\": {\n" +
                         "    \"op\": \"JournalSource\",\n" +
@@ -320,7 +323,7 @@ public class JoinQueryTest extends AbstractOptimiserTest {
     @Test
     public void testDuplicateAlias() throws Exception {
         try {
-            assertPlan("", "customers a" +
+            assertPlan2("", "customers a" +
                     " cross join orders a");
             Assert.fail("Exception expected");
         } catch (ParserException e) {
@@ -331,9 +334,29 @@ public class JoinQueryTest extends AbstractOptimiserTest {
 
     @Test
     public void testDuplicateJournals() throws Exception {
-        assertPlan("+ 0[ cross ] customers\n" +
-                        "+ 1[ cross ] customers\n" +
-                        "\n",
+        assertPlan2("{\n" +
+                        "  \"op\": \"CrossJoinRecordSource\",\n" +
+                        "  \"master\": {\n" +
+                        "    \"op\": \"JournalSource\",\n" +
+                        "    \"psrc\": {\n" +
+                        "      \"op\": \"JournalPartitionSource\",\n" +
+                        "      \"journal\": \"customers\"\n" +
+                        "    },\n" +
+                        "    \"rsrc\": {\n" +
+                        "      \"op\": \"AllRowSource\"\n" +
+                        "    }\n" +
+                        "  },\n" +
+                        "  \"slave\": {\n" +
+                        "    \"op\": \"JournalSource\",\n" +
+                        "    \"psrc\": {\n" +
+                        "      \"op\": \"JournalPartitionSource\",\n" +
+                        "      \"journal\": \"customers\"\n" +
+                        "    },\n" +
+                        "    \"rsrc\": {\n" +
+                        "      \"op\": \"AllRowSource\"\n" +
+                        "    }\n" +
+                        "  }\n" +
+                        "}",
                 "customers" +
                         " cross join customers");
     }
@@ -453,7 +476,7 @@ public class JoinQueryTest extends AbstractOptimiserTest {
     @Test
     public void testInvalidAlias() throws Exception {
         try {
-            assertPlan("", "orders join customers on orders.customerId = c.customerId");
+            assertPlan2("", "orders join customers on orders.customerId = c.customerId");
             Assert.fail("Exception expected");
         } catch (ParserException e) {
             Assert.assertEquals(45, QueryError.getPosition());
@@ -464,7 +487,7 @@ public class JoinQueryTest extends AbstractOptimiserTest {
     @Test
     public void testInvalidColumn() throws Exception {
         try {
-            assertPlan("", "orders join customers on customerIdx = customerId");
+            assertPlan2("", "orders join customers on customerIdx = customerId");
             Assert.fail("Exception expected");
         } catch (ParserException e) {
             Assert.assertEquals(25, QueryError.getPosition());
@@ -514,7 +537,7 @@ public class JoinQueryTest extends AbstractOptimiserTest {
     @Test
     public void testInvalidTableName() throws Exception {
         try {
-            assertPlan("", "orders join customer on customerId = customerId");
+            assertPlan2("", "orders join customer on customerId = customerId");
             Assert.fail("Exception expected");
         } catch (ParserException e) {
             Assert.assertEquals(12, QueryError.getPosition());