You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by vo...@apache.org on 2019/02/13 08:53:16 UTC

[ignite] branch master updated: IGNITE-11280: SQL: common caching logic for all statement types. This closes #6091.

This is an automated email from the ASF dual-hosted git repository.

vozerov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git


The following commit(s) were added to refs/heads/master by this push:
     new ed69632  IGNITE-11280: SQL: common caching logic for all statement types. This closes #6091.
ed69632 is described below

commit ed6963269046fd0cd11dc9f8b45c3d3cd464bfef
Author: devozerov <vo...@gridgain.com>
AuthorDate: Wed Feb 13 11:53:06 2019 +0300

    IGNITE-11280: SQL: common caching logic for all statement types. This closes #6091.
---
 .../jdbc/thin/JdbcThinLocalQueriesSelfTest.java    |   7 --
 .../internal/processors/query/h2/QueryParser.java  | 111 +++++++++------------
 .../processors/query/h2/QueryParserCacheEntry.java |  53 ++++++----
 .../query/h2/QueryParserResultSelect.java          |   3 +-
 .../query/IgniteCachelessQueriesSelfTest.java      |   2 +-
 5 files changed, 85 insertions(+), 91 deletions(-)

diff --git a/modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinLocalQueriesSelfTest.java b/modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinLocalQueriesSelfTest.java
index ac81a71..d460dbc 100644
--- a/modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinLocalQueriesSelfTest.java
+++ b/modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinLocalQueriesSelfTest.java
@@ -20,9 +20,7 @@ package org.apache.ignite.jdbc.thin;
 import java.sql.Connection;
 import java.sql.SQLException;
 import java.util.List;
-import java.util.Map;
 import org.apache.ignite.internal.util.typedef.F;
-import org.apache.ignite.internal.util.typedef.internal.U;
 import org.junit.Test;
 
 /**
@@ -65,11 +63,6 @@ public class JdbcThinLocalQueriesSelfTest extends JdbcThinAbstractSelfTest {
                 "p.companyid = c.id");
 
             assertEqualsCollections(F.asList(2, "John", "Apple"), res.get(0));
-
-            Map twoStepCache = U.field((Object)U.field(grid(0).context().query().getIndexing(), "parser"), "cache");
-
-            // No two step queries cached => local select.
-            assertEquals(0, twoStepCache.size());
         }
     }
 }
diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParser.java b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParser.java
index 0b0baf0..7ab2696 100644
--- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParser.java
+++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParser.java
@@ -123,34 +123,36 @@ public class QueryParser {
      */
     private QueryParserResult parse0(String schemaName, SqlFieldsQuery qry) {
         // First, let's check if we already have a two-step query for this statement...
-        QueryParserCacheKey cachedQryKey = new QueryParserCacheKey(
+        QueryParserCacheKey cachedKey = new QueryParserCacheKey(
             schemaName,
             qry.getSql(),
             qry.isCollocated(),
             qry.isDistributedJoins(),
             qry.isEnforceJoinOrder(),
-            qry.isLocal());
+            qry.isLocal()
+        );
 
-        QueryParserCacheEntry cachedQry = cache.get(cachedQryKey);
+        QueryParserCacheEntry cached = cache.get(cachedKey);
 
-        if (cachedQry != null) {
-            QueryParserResultSelect select = new QueryParserResultSelect(
-                cachedQry.query(),
-                cachedQry.meta(),
-                null
-            );
-
-            return new QueryParserResult(qry, null, select, null, null);
-        }
+        if (cached != null)
+            return new QueryParserResult(qry, null, cached.select(), cached.dml(), cached.command());
 
         // Try parting as native command.
         QueryParserResult parseRes = parseNative(schemaName, qry);
 
-        if (parseRes != null)
-            return parseRes;
+        // Otherwise parse with H2.
+        if (parseRes == null)
+            parseRes = parseH2(schemaName, qry);
+
+        // Add to cache if not multi-statement.
+        if (parseRes.remainingQuery() == null) {
+            cached = new QueryParserCacheEntry(parseRes.select(), parseRes.dml(), parseRes.command());
+
+            cache.put(cachedKey, cached);
+        }
 
-        // Parse with H2.
-        return parseH2(schemaName, qry);
+        // Done.
+        return parseRes;
     }
 
     /**
@@ -371,56 +373,39 @@ public class QueryParser {
         }
 
         // Only distirbuted SELECT are possible at this point.
-        QueryParserCacheKey cachedQryKey = new QueryParserCacheKey(
-            schemaName,
-            qry.getSql(),
-            qry.isCollocated(),
-            qry.isDistributedJoins(),
-            qry.isEnforceJoinOrder(),
-            qry.isLocal()
-        );
-
-        QueryParserCacheEntry cachedQry = cache.get(cachedQryKey);
+        try {
+            GridCacheTwoStepQuery twoStepQry = GridSqlQuerySplitter.split(
+                connMgr.connectionForThread().connection(newQry.getSchema()),
+                prepared,
+                newQry.getArgs(),
+                newQry.isCollocated(),
+                newQry.isDistributedJoins(),
+                newQry.isEnforceJoinOrder(),
+                newQry.isLocal(),
+                idx
+            );
 
-        if (cachedQry == null) {
-            try {
-                GridCacheTwoStepQuery twoStepQry = GridSqlQuerySplitter.split(
-                    connMgr.connectionForThread().connection(newQry.getSchema()),
-                    prepared,
-                    newQry.getArgs(),
-                    newQry.isCollocated(),
-                    newQry.isDistributedJoins(),
-                    newQry.isEnforceJoinOrder(),
-                    newQry.isLocal(),
-                    idx
-                );
-
-                List<GridQueryFieldMetadata> meta = H2Utils.meta(stmt.getMetaData());
-
-                cachedQry = new QueryParserCacheEntry(meta, twoStepQry);
-
-                if (remainingQry == null && !twoStepQry.explain())
-                    cache.putIfAbsent(cachedQryKey, cachedQry);
-            }
-            catch (IgniteCheckedException e) {
-                throw new IgniteSQLException("Failed to bind parameters: [qry=" + newQry.getSql() + ", params=" +
-                    Arrays.deepToString(newQry.getArgs()) + "]", IgniteQueryErrorCode.PARSING, e);
-            }
-            catch (SQLException e) {
-                throw new IgniteSQLException(e);
-            }
-            finally {
-                U.close(stmt, log);
-            }
-        }
+            List<GridQueryFieldMetadata> meta = H2Utils.meta(stmt.getMetaData());
 
-        QueryParserResultSelect select = new QueryParserResultSelect(
-            cachedQry.query(),
-            cachedQry.meta(),
-            prepared
-        );
+            QueryParserResultSelect select = new QueryParserResultSelect(
+                twoStepQry,
+                meta,
+                prepared
+            );
 
-        return new QueryParserResult(newQry, remainingQry, select, null, null);
+            return new QueryParserResult(newQry, remainingQry, select, null, null);
+        }
+        catch (IgniteCheckedException e) {
+            throw new IgniteSQLException("Failed to bind parameters: [qry=" + newQry.getSql() + ", params=" +
+                Arrays.deepToString(newQry.getArgs()) + "]", IgniteQueryErrorCode.PARSING, e);
+        }
+        catch (SQLException e) {
+            throw new IgniteSQLException(e);
+        }
+        finally {
+            // TODO: Leak if returned earlier. Will be fixed in https://issues.apache.org/jira/browse/IGNITE-11279
+            U.close(stmt, log);
+        }
     }
 
     /**
diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParserCacheEntry.java b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParserCacheEntry.java
index 2531379..041476c 100644
--- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParserCacheEntry.java
+++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParserCacheEntry.java
@@ -17,43 +17,58 @@
 
 package org.apache.ignite.internal.processors.query.h2;
 
-import org.apache.ignite.internal.processors.cache.query.GridCacheTwoStepQuery;
-import org.apache.ignite.internal.processors.query.GridQueryFieldMetadata;
 import org.apache.ignite.internal.util.typedef.internal.S;
-
-import java.util.List;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * Cached two-step query.
  */
 public class QueryParserCacheEntry {
-    /** */
-    private final List<GridQueryFieldMetadata> meta;
+    /** Select. */
+    private final QueryParserResultSelect select;
+
+    /** DML. */
+    private final QueryParserResultDml dml;
+
+    /** Command. */
+    private final QueryParserResultCommand cmd;
 
-    /** */
-    private final GridCacheTwoStepQuery twoStepQry;
+    /**
+     * Constructor.
+     *
+     * @param select SELECT.
+     * @param dml DML.
+     * @param cmd Command.
+     */
+    public QueryParserCacheEntry(
+        @Nullable QueryParserResultSelect select,
+        @Nullable QueryParserResultDml dml,
+        @Nullable QueryParserResultCommand cmd
+    ) {
+        this.select = select;
+        this.dml = dml;
+        this.cmd = cmd;
+    }
 
     /**
-     * @param meta Fields metadata.
-     * @param twoStepQry Query.
+     * @return SELECT.
      */
-    public QueryParserCacheEntry(List<GridQueryFieldMetadata> meta, GridCacheTwoStepQuery twoStepQry) {
-        this.meta = meta;
-        this.twoStepQry = twoStepQry;
+    @Nullable public QueryParserResultSelect select() {
+        return select;
     }
 
     /**
-     * @return Fields metadata.
+     * @return DML.
      */
-    public List<GridQueryFieldMetadata> meta() {
-        return meta;
+    @Nullable public QueryParserResultDml dml() {
+        return dml;
     }
 
     /**
-     * @return Query.
+     * @return Command.
      */
-    public GridCacheTwoStepQuery query() {
-        return twoStepQry;
+    @Nullable public QueryParserResultCommand command() {
+        return cmd;
     }
 
     /** {@inheritDoc} */
diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParserResultSelect.java b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParserResultSelect.java
index a5787c2..3805fba 100644
--- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParserResultSelect.java
+++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParserResultSelect.java
@@ -26,6 +26,7 @@ import java.util.List;
 /**
  * Parsing result for SELECT.
  */
+@SuppressWarnings("AssignmentOrReturnOfFieldWithMutableType")
 public class QueryParserResultSelect {
     /** Two-step query, or {@code} null if this result is for local query. */
     private final GridCacheTwoStepQuery twoStepQry;
@@ -49,7 +50,7 @@ public class QueryParserResultSelect {
     /**
      * @return Two-step query, or {@code} null if this result is for local query.
      */
-    GridCacheTwoStepQuery twoStepQuery() {
+    public GridCacheTwoStepQuery twoStepQuery() {
         return twoStepQry;
     }
 
diff --git a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteCachelessQueriesSelfTest.java b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteCachelessQueriesSelfTest.java
index 1d43835..c8ba516 100644
--- a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteCachelessQueriesSelfTest.java
+++ b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteCachelessQueriesSelfTest.java
@@ -326,7 +326,7 @@ public class IgniteCachelessQueriesSelfTest extends GridCommonAbstractTest {
 
         QueryParserCacheEntry q = m.values().iterator().next();
 
-        return q.query();
+        return q.select().twoStepQuery();
     }
 
     /**