ignite-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From se...@apache.org
Subject [1/2] ignite git commit: master - fixes splitter push down subquery expressions in SELECT and WHERE clauses
Date Wed, 14 Jun 2017 17:04:09 GMT
Repository: ignite
Updated Branches:
  refs/heads/master b6ad6c055 -> a79724dfd


master - fixes splitter push down subquery expressions in SELECT and WHERE clauses


Project: http://git-wip-us.apache.org/repos/asf/ignite/repo
Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/70eed754
Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/70eed754
Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/70eed754

Branch: refs/heads/master
Commit: 70eed75422ea50a7bb9dbe539e2b9a62458e57fd
Parents: b843f07
Author: Sergi Vladykin <sergi.vladykin@gmail.com>
Authored: Wed Jun 14 19:59:53 2017 +0300
Committer: Sergi Vladykin <sergi.vladykin@gmail.com>
Committed: Wed Jun 14 19:59:53 2017 +0300

----------------------------------------------------------------------
 .../processors/query/h2/IgniteH2Indexing.java   |   4 +-
 .../query/h2/sql/GridSqlQuerySplitter.java      | 145 ++++++++++++++++---
 .../query/IgniteSqlSplitterSelfTest.java        |  74 +++++++++-
 3 files changed, 197 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ignite/blob/70eed754/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java
----------------------------------------------------------------------
diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java
index e7f93c5..fc3b28f 100644
--- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java
+++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java
@@ -192,6 +192,7 @@ public class IgniteH2Indexing implements GridQueryIndexing {
         System.setProperty("h2.objectCache", "false");
         System.setProperty("h2.serializeJavaObject", "false");
         System.setProperty("h2.objectCacheMaxPerElementSize", "0"); // Avoid ValueJavaObject
caching.
+        System.setProperty("h2.optimizeTwoEquals", "false"); // Makes splitter fail on subqueries
in WHERE.
     }
 
     /** Default DB options. */
@@ -991,7 +992,8 @@ public class IgniteH2Indexing implements GridQueryIndexing {
 
                 // Add SQL explain result message into log.
                 String longMsg = "Query execution is too long [time=" + time + " ms, sql='"
+ sql + '\'' +
-                    ", plan=" + U.nl() + plan.getString(1) + U.nl() + ", parameters=" + params
+ "]";
+                    ", plan=" + U.nl() + plan.getString(1) + U.nl() + ", parameters=" +
+                    (params == null ? "[]" : Arrays.deepToString(params.toArray())) + "]";
 
                 LT.warn(log, longMsg, msg);
             }

http://git-wip-us.apache.org/repos/asf/ignite/blob/70eed754/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuerySplitter.java
----------------------------------------------------------------------
diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuerySplitter.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuerySplitter.java
index 25d97fc..aebf596 100644
--- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuerySplitter.java
+++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuerySplitter.java
@@ -97,6 +97,12 @@ public class GridSqlQuerySplitter {
     private static final String UNIQUE_TABLE_ALIAS_SUFFIX = "__Z";
 
     /** */
+    private static final String EXPR_ALIAS_PREFIX = "__X";
+
+    /** */
+    private int nextExprAliasId;
+
+    /** */
     private int nextTblAliasId;
 
     /** */
@@ -302,8 +308,12 @@ public class GridSqlQuerySplitter {
         // Get back the updated query from the fake parent. It will be our reduce query.
         qry = fakeQryPrnt.subquery();
 
+        String rdcQry = qry.getSQL();
+
+        checkNoDataTablesInReduceQuery(qry, rdcQry);
+
         // Setup a resulting reduce query.
-        rdcSqlQry = new GridCacheSqlQuery(qry.getSQL());
+        rdcSqlQry = new GridCacheSqlQuery(rdcQry);
         rdcQrySimple = qry.simpleQuery();
 
         setupParameters(rdcSqlQry, qry, params);
@@ -346,6 +356,21 @@ public class GridSqlQuerySplitter {
     }
 
     /**
+     * @param ast Reduce query AST.
+     * @param rdcQry Reduce query string.
+     */
+    private static void checkNoDataTablesInReduceQuery(GridSqlAst ast, String rdcQry) {
+        if (ast instanceof GridSqlTable) {
+            if (((GridSqlTable)ast).dataTable() != null)
+                throw new IgniteException("Failed to generate REDUCE query. Data table found:
" + ast.getSQL() + " \n" + rdcQry);
+        }
+        else {
+            for (int i = 0; i < ast.size(); i++)
+                checkNoDataTablesInReduceQuery(ast.child(i), rdcQry);
+        }
+    }
+
+    /**
      * @param expr Expression.
      * @return {@code true} If the expression contains pushed down columns.
      */
@@ -658,6 +683,27 @@ public class GridSqlQuerySplitter {
     }
 
     /**
+     * @param select Select.
+     * @param aliases Table aliases in FROM.
+     */
+    private static void collectFromAliases(GridSqlSelect select, Set<GridSqlAlias>
aliases) {
+        GridSqlAst from = select.from();
+
+        if (from == null)
+            return;
+
+        while (from instanceof GridSqlJoin) {
+            GridSqlElement right = ((GridSqlJoin)from).rightTable();
+
+            aliases.add((GridSqlAlias)right);
+
+            from = ((GridSqlJoin)from).leftTable();
+        }
+
+        aliases.add((GridSqlAlias)from);
+    }
+
+    /**
      * @param qrym Query model.
      * @param begin The first child model in range to push down.
      * @param end The last child model in range to push down.
@@ -900,17 +946,32 @@ public class GridSqlQuerySplitter {
         for (int i = 0; i < select.allColumns(); i++) {
             GridSqlAst expr = select.column(i);
 
-            if (expr instanceof GridSqlColumn) {
-                // If this is a column with no expression and without alias, we need replace
it with an alias,
+            if (!(expr instanceof GridSqlAlias)) {
+                // If this is an expression without alias, we need replace it with an alias,
                 // because in the wrapQuery we will generate unique aliases for all the columns
to avoid duplicates
                 // and all the columns in the will be replaced.
-                expr = alias(((GridSqlColumn)expr).columnName(), expr);
+                String alias;
+
+                if (expr instanceof GridSqlColumn)
+                    alias = ((GridSqlColumn)expr).columnName();
+                else
+                    alias = EXPR_ALIAS_PREFIX + i;
+
+                expr = alias(alias, expr);
 
                 select.setColumn(i, expr);
             }
 
-            for (int c = 0; c < expr.size(); c++)
-                pushDownColumnsInExpression(tblAliases, cols, wrapAlias, expr, c);
+            assert expr instanceof GridSqlAlias;
+
+            if (isAllRelatedToTables(tblAliases, GridSqlQuerySplitter.<GridSqlAlias>newIdentityHashSet(),
expr)) {
+                // Push down the whole expression.
+                pushDownColumn(tblAliases, cols, wrapAlias, expr, 0);
+            }
+            else {
+                // Push down each column separately.
+                pushDownColumnsInExpression(tblAliases, cols, wrapAlias, expr, 0);
+            }
         }
     }
 
@@ -953,31 +1014,40 @@ public class GridSqlQuerySplitter {
         GridSqlAst prnt,
         int childIdx
     ) {
-        GridSqlColumn col = prnt.child(childIdx);
+        GridSqlAst expr = prnt.child(childIdx);
 
-        // It must always be unique table alias.
-        GridSqlAlias tblAlias = (GridSqlAlias)col.expressionInFrom();
+        String uniqueColAlias;
 
-        assert tblAlias != null; // The query is normalized.
+        if (expr instanceof GridSqlColumn) {
+            GridSqlColumn col = prnt.child(childIdx);
 
-        if (!tblAliases.contains(tblAlias))
-            return;
+            // It must always be unique table alias.
+            GridSqlAlias tblAlias = (GridSqlAlias)col.expressionInFrom();
 
-        GridSqlType resType = col.resultType();
-        String uniqueColAlias = uniqueColumnAlias(col);
+            assert tblAlias != null; // The query is normalized.
+
+            if (!tblAliases.contains(tblAlias))
+                return; // Unrelated column, nothing to do.
+
+            uniqueColAlias = uniquePushDownColumnAlias(col);
+        }
+        else {
+            uniqueColAlias = EXPR_ALIAS_PREFIX + nextExprAliasId++ + "__" + ((GridSqlAlias)prnt).alias();
+        }
+
+        GridSqlType resType = expr.resultType();
         GridSqlAlias colAlias = cols.get(uniqueColAlias);
 
         if (colAlias == null) {
-            colAlias = alias(uniqueColAlias, col);
+            colAlias = alias(uniqueColAlias, expr);
 
             // We have this map to avoid column duplicates in wrap query.
             cols.put(uniqueColAlias, colAlias);
 
-            if (!pushedDownCols.add(uniqueColAlias)) // Must be globally unique.
-                throw new IllegalStateException(uniqueColAlias);
+            pushedDownCols.add(uniqueColAlias);
         }
 
-        col = column(uniqueColAlias);
+        GridSqlColumn col = column(uniqueColAlias);
         // col.tableAlias(wrapAlias.alias());
         col.expressionInFrom(wrapAlias);
         col.resultType(resType);
@@ -989,10 +1059,27 @@ public class GridSqlQuerySplitter {
      * @param col Column.
      * @return Unique column alias based on generated unique table alias.
      */
-    private String uniqueColumnAlias(GridSqlColumn col) {
+    private String uniquePushDownColumnAlias(GridSqlColumn col) {
+        String colName = col.columnName();
+
+        if (pushedDownCols.contains(colName))
+            return colName; // Already pushed down unique alias.
+
         GridSqlAlias uniqueTblAlias = (GridSqlAlias)col.expressionInFrom();
 
-        return uniqueTblAlias.alias() + "__" + col.columnName();
+        return uniquePushDownColumnAlias(uniqueTblAlias.alias(), colName);
+    }
+
+    /**
+     * @param uniqueTblAlias Unique table alias.
+     * @param colName Column name.
+     * @return Unique column alias based on generated unique table alias.
+     */
+    private static String uniquePushDownColumnAlias(String uniqueTblAlias, String colName)
{
+        assert !F.isEmpty(uniqueTblAlias);
+        assert !F.isEmpty(colName);
+
+        return uniqueTblAlias + "__" + colName;
     }
 
     /**
@@ -1020,7 +1107,9 @@ public class GridSqlQuerySplitter {
             AndCondition c = andConditions.get(i);
             GridSqlAst condition = c.ast();
 
-            if (isAllRelatedToTables(tblAliases, condition)) {
+            if (isAllRelatedToTables(tblAliases,
+                GridSqlQuerySplitter.<GridSqlAlias>newIdentityHashSet(),
+                condition)) {
                 if (!isTrue(condition)) {
                     // Replace the original condition with `true` and move it to the wrap
query.
                     c.prnt.child(c.childIdx, TRUE);
@@ -1042,20 +1131,28 @@ public class GridSqlQuerySplitter {
 
     /**
      * @param tblAliases Table aliases for push down.
+     * @param locSubQryTblAliases Local subquery tables.
      * @param ast AST.
      * @return {@code true} If all the columns in the given expression are related to the
given tables.
      */
     @SuppressWarnings("SuspiciousMethodCalls")
-    private boolean isAllRelatedToTables(Set<GridSqlAlias> tblAliases, GridSqlAst ast)
{
+    private boolean isAllRelatedToTables(Set<GridSqlAlias> tblAliases, Set<GridSqlAlias>
locSubQryTblAliases, GridSqlAst ast) {
         if (ast instanceof GridSqlColumn) {
             GridSqlColumn col = (GridSqlColumn)ast;
 
-            if (!tblAliases.contains(col.expressionInFrom()))
+            // The column must be related to the given list of tables.
+            if (!tblAliases.contains(col.expressionInFrom()) &&
+                // Or must be rated to some local expression subquery.
+                !locSubQryTblAliases.contains(col.expressionInFrom()))
                 return false;
         }
         else {
+            // If it is a subquery, collect all the table aliases to ignore them later.
+            if (ast instanceof GridSqlSelect)
+                collectFromAliases((GridSqlSelect)ast, locSubQryTblAliases);
+
             for (int i = 0; i < ast.size(); i++) {
-                if (!isAllRelatedToTables(tblAliases, ast.child(i)))
+                if (!isAllRelatedToTables(tblAliases, locSubQryTblAliases, ast.child(i)))
                     return false;
             }
         }

http://git-wip-us.apache.org/repos/asf/ignite/blob/70eed754/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java
----------------------------------------------------------------------
diff --git a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java
b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java
index 6c61988..a38a61f 100644
--- a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java
+++ b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java
@@ -165,8 +165,80 @@ public class IgniteSqlSplitterSelfTest extends GridCommonAbstractTest
{
 
     /**
      */
+    public void _testMergeJoin() {
+        IgniteCache<Integer, Org> c = ignite(CLIENT).getOrCreateCache(cacheConfig("org",
true,
+            Integer.class, Org.class));
+
+        try {
+            String qry = "select o1.* from Org o1, " +
+                "(select max(o.name) as name, o.id from Org o group by o.id) o2 " +
+                "where o1.id = o2.id";
+
+            List<List<?>> plan = c.query(new SqlFieldsQuery("explain " + qry)
+                .setEnforceJoinOrder(true)).getAll();
+
+            X.println("Plan: " + plan);
+
+            String map0 = (String)plan.get(0).get(0);
+            String map1 = (String)plan.get(1).get(0);
+            String rdc = (String)plan.get(2).get(0);
+
+            assertTrue(map0.contains("ORDER BY"));
+            assertTrue(map1.contains("ORDER BY"));
+            assertEquals(3, rdc.split("merge_sorted").length);
+        }
+        finally {
+            c.destroy();
+        }
+    }
+
+    public void testPushDownSubquery() {
+        IgniteCache<Integer, Person> c = ignite(CLIENT).getOrCreateCache(cacheConfig("ps",
true,
+            Integer.class, Person.class));
+
+        try {
+            String subqry = "(select max(p.id) as id, p.depId from Person p group by p.depId)";
+
+            // Subquery in where clause.
+            String qry = "select 1 from Person p0 join " + subqry + " p1 on p0.id = p1.id
where p0.id = " +
+                " (select p.id from Person p where p.id = p0.id)";
+
+            assertEquals(0, c.query(new SqlFieldsQuery(qry)).getAll().size());
+
+            List<List<?>> plan = c.query(new SqlFieldsQuery("explain " + qry)).getAll();
+
+            X.println(" Plan: " + plan);
+
+            assertEquals(3, plan.size());
+
+            String rdc = (String)plan.get(2).get(0);
+
+            assertFalse(rdc.contains("PERSON"));
+
+            qry = "select (select p.id from Person p where p.id = p0.id) from Person p0 join
" +
+                subqry + " p1 on p0.id = p1.id";
+
+            assertEquals(0, c.query(new SqlFieldsQuery(qry)).getAll().size());
+
+            plan = c.query(new SqlFieldsQuery("explain " + qry)).getAll();
+
+            X.println(" Plan: " + plan);
+
+            assertEquals(3, plan.size());
+
+            rdc = (String)plan.get(2).get(0);
+
+            assertFalse(rdc.contains("PERSON"));
+        }
+        finally {
+            c.destroy();
+        }
+    }
+
+    /**
+     */
     public void testPushDown() {
-        IgniteCache<Integer, Person> c = ignite(0).getOrCreateCache(cacheConfig("ps",
true,
+        IgniteCache<Integer, Person> c = ignite(CLIENT).getOrCreateCache(cacheConfig("ps",
true,
             Integer.class, Person.class));
 
         try {


Mime
View raw message