You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by tl...@apache.org on 2021/03/04 13:08:35 UTC
[ignite] branch sql-calcite updated: IGNITE-14163 fix an issue with
range predicate serialization (#8851)
This is an automated email from the ASF dual-hosted git repository.
tledkov pushed a commit to branch sql-calcite
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/sql-calcite by this push:
new 123a9b6 IGNITE-14163 fix an issue with range predicate serialization (#8851)
123a9b6 is described below
commit 123a9b66795c558e47e6647482e6972789abce5d
Author: korlov42 <ko...@gridgain.com>
AuthorDate: Thu Mar 4 16:07:51 2021 +0300
IGNITE-14163 fix an issue with range predicate serialization (#8851)
---
.../query/calcite/CalciteQueryProcessor.java | 4 ++++
.../processors/query/calcite/externalize/RelJson.java | 13 +++++++++++++
.../query/calcite/externalize/RelJsonReader.java | 4 +++-
.../query/calcite/externalize/RelJsonWriter.java | 10 +++++++---
.../CalciteBasicSecondaryIndexIntegrationTest.java | 18 ++++++++++++++++++
.../processors/odbc/jdbc/JdbcRequestHandler.java | 5 +++--
.../internal/processors/query/GridQueryProcessor.java | 19 ++++++++++++++-----
7 files changed, 62 insertions(+), 11 deletions(-)
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessor.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessor.java
index 1f09f08..53184da 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessor.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessor.java
@@ -71,6 +71,10 @@ public class CalciteQueryProcessor extends GridProcessorAdapter implements Query
.executor(EXECUTOR)
.sqlToRelConverterConfig(SqlToRelConverter.config()
.withTrimUnusedFields(true)
+ // currently SqlToRelConverter creates not optimal plan for both optimization and execution
+ // so it's better to disable such rewriting right now
+ // TODO: remove this after IGNITE-14277
+ .withInSubQueryThreshold(Integer.MAX_VALUE)
.withDecorrelationEnabled(true))
.parserConfig(
SqlParser.config()
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJson.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJson.java
index b91e3e0..d33e838 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJson.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJson.java
@@ -28,6 +28,7 @@ import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
+
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
@@ -65,6 +66,7 @@ import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.rex.RexOver;
import org.apache.calcite.rex.RexSlot;
+import org.apache.calcite.rex.RexUtil;
import org.apache.calcite.rex.RexVariable;
import org.apache.calcite.rex.RexWindow;
import org.apache.calcite.rex.RexWindowBound;
@@ -113,6 +115,9 @@ import org.apache.ignite.internal.util.typedef.internal.U;
@SuppressWarnings({"rawtypes", "unchecked"})
class RelJson {
/** */
+ private final RelOptCluster cluster;
+
+ /** */
@SuppressWarnings("PublicInnerClass") @FunctionalInterface
public static interface RelFactory extends Function<RelInput, RelNode> {
/** {@inheritDoc} */
@@ -220,6 +225,11 @@ class RelJson {
"org.apache.calcite.adapter.jdbc.JdbcRules$");
/** */
+ RelJson(RelOptCluster cluster) {
+ this.cluster = cluster;
+ }
+
+ /** */
Function<RelInput, RelNode> factory(String type) {
return FACTORIES_CACHE.getUnchecked(type);
}
@@ -716,6 +726,9 @@ class RelJson {
/** */
private Object toJson(RexNode node) {
+ // removes calls to SEARCH and the included Sarg and converts them to comparisons
+ node = RexUtil.expandSearch(cluster.getRexBuilder(), null, node);
+
Map<String, Object> map;
switch (node.getKind()) {
case FIELD_ACCESS:
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJsonReader.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJsonReader.java
index 4709db3..3461d89 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJsonReader.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJsonReader.java
@@ -63,7 +63,7 @@ public class RelJsonReader {
private final RelOptSchema relOptSchema;
/** */
- private final RelJson relJson = new RelJson();
+ private final RelJson relJson;
/** */
private final Map<String, RelNode> relMap = new LinkedHashMap<>();
@@ -82,6 +82,8 @@ public class RelJsonReader {
public RelJsonReader(RelOptCluster cluster, RelOptSchema relOptSchema) {
this.cluster = cluster;
this.relOptSchema = relOptSchema;
+
+ relJson = new RelJson(cluster);
}
/** */
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJsonWriter.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJsonWriter.java
index aa35b27..eade491 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJsonWriter.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/externalize/RelJsonWriter.java
@@ -22,9 +22,11 @@ import java.util.ArrayList;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
+
import com.fasterxml.jackson.core.util.DefaultPrettyPrinter;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectWriter;
+import org.apache.calcite.plan.RelOptCluster;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.RelWriter;
import org.apache.calcite.sql.SqlExplainLevel;
@@ -42,7 +44,7 @@ public class RelJsonWriter implements RelWriter {
private static final boolean PRETTY_PRINT = IgniteSystemProperties.getBoolean("IGNITE_CALCITE_REL_JSON_PRETTY_PRINT", false);
/** */
- private final RelJson relJson = new RelJson();
+ private final RelJson relJson;
/** */
private final List<Object> relList = new ArrayList<>();
@@ -61,15 +63,17 @@ public class RelJsonWriter implements RelWriter {
/** */
public static String toJson(RelNode rel) {
- RelJsonWriter writer = new RelJsonWriter(PRETTY_PRINT);
+ RelJsonWriter writer = new RelJsonWriter(rel.getCluster(), PRETTY_PRINT);
rel.explain(writer);
return writer.asString();
}
/** */
- public RelJsonWriter(boolean pretty) {
+ public RelJsonWriter(RelOptCluster cluster, boolean pretty) {
this.pretty = pretty;
+
+ relJson = new RelJson(cluster);
}
/** {@inheritDoc} */
diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CalciteBasicSecondaryIndexIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CalciteBasicSecondaryIndexIntegrationTest.java
index 04fe3d9..0c9de8d 100644
--- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CalciteBasicSecondaryIndexIntegrationTest.java
+++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CalciteBasicSecondaryIndexIntegrationTest.java
@@ -899,6 +899,24 @@ public class CalciteBasicSecondaryIndexIntegrationTest extends GridCommonAbstrac
.check();
}
+ /**
+ * Test verifies that ranges would be serialized and desirialized without any errors.
+ */
+ @Test
+ public void testSelectWithRanges() {
+ String sql = "select depId from Developer " +
+ "where depId in (1,2,3,5,6,7,9,10,13,14,15,18,19,20,21,22,23,24,25,26,27,28,30,31,32,33) " +
+ " or depId between 7 and 8 order by depId limit 5";
+
+ assertQuery(sql)
+ .returns(1)
+ .returns(2)
+ .returns(2)
+ .returns(3)
+ .returns(5)
+ .check();
+ }
+
/** */
private QueryChecker assertQuery(String qry) {
return new QueryChecker(qry) {
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/odbc/jdbc/JdbcRequestHandler.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/odbc/jdbc/JdbcRequestHandler.java
index e67e70c..769e357 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/processors/odbc/jdbc/JdbcRequestHandler.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/odbc/jdbc/JdbcRequestHandler.java
@@ -31,6 +31,7 @@ import java.util.SortedSet;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
+
import javax.cache.configuration.Factory;
import org.apache.ignite.IgniteCheckedException;
import org.apache.ignite.IgniteLogger;
@@ -115,7 +116,7 @@ import static org.apache.ignite.internal.processors.odbc.jdbc.JdbcRequest.QRY_CL
import static org.apache.ignite.internal.processors.odbc.jdbc.JdbcRequest.QRY_EXEC;
import static org.apache.ignite.internal.processors.odbc.jdbc.JdbcRequest.QRY_FETCH;
import static org.apache.ignite.internal.processors.odbc.jdbc.JdbcRequest.QRY_META;
-import static org.apache.ignite.internal.processors.query.GridQueryProcessor.H2_REDIRECTION_RULES;
+import static org.apache.ignite.internal.processors.query.GridQueryProcessor.shouldRedirectToCalcite;
/**
* JDBC request handler.
@@ -786,7 +787,7 @@ public class JdbcRequestHandler implements ClientListenerRequestHandler {
/** */
private List<FieldsQueryCursor<List<?>>> querySqlFields(SqlFieldsQueryEx qry, GridQueryCancel cancel) {
if (experimentalQueryEngine != null) {
- if (!H2_REDIRECTION_RULES.matcher(qry.getSql()).find())
+ if (shouldRedirectToCalcite(qry.getSql()))
return experimentalQueryEngine.query(QueryContext.of(qry, cancel), qry.getSchema(), qry.getSql(), qry.getArgs());
}
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java
index 49ab287..59f9488 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java
@@ -35,6 +35,7 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Pattern;
+
import javax.cache.Cache;
import javax.cache.CacheException;
import org.apache.ignite.IgniteCheckedException;
@@ -165,6 +166,10 @@ public class GridQueryProcessor extends GridProcessorAdapter {
/** */
private static final ThreadLocal<AffinityTopologyVersion> requestTopVer = new ThreadLocal<>();
+ /** Patter to test incoming query to decide whether this query should be executed with Calcite or H2. */
+ private static final Pattern IS_SELECT_OR_EXPLAIN_PATTERN =
+ Pattern.compile("^\\s*(select|explain plan for)", CASE_INSENSITIVE);
+
/** For tests. */
public static Class<? extends GridQueryIndexing> idxCls;
@@ -245,10 +250,6 @@ public class GridQueryProcessor extends GridProcessorAdapter {
/** Experimental calcite query engine. */
private QueryEngine experimentalQueryEngine;
- /** h2 redirection stub. */
- public static final Pattern H2_REDIRECTION_RULES =
- Pattern.compile("\\s*((create|drop)\\s*(table|index)|alter\\s*table)", CASE_INSENSITIVE);
-
/** @see IgniteSystemProperties#IGNITE_EXPERIMENTAL_SQL_ENGINE */
public static final boolean DFLT_IGNITE_EXPERIMENTAL_SQL_ENGINE = false;
@@ -2837,7 +2838,7 @@ public class GridQueryProcessor extends GridProcessorAdapter {
throw new CacheException("Execution of local SqlFieldsQuery on client node disallowed.");
if (experimentalQueryEngine != null && useExperimentalSqlEngine) {
- if (!H2_REDIRECTION_RULES.matcher(qry.getSql()).find())
+ if (shouldRedirectToCalcite(qry.getSql()))
return experimentalQueryEngine.query(QueryContext.of(qry), qry.getSchema(), qry.getSql(), X.EMPTY_OBJECT_ARRAY);
}
@@ -3630,6 +3631,14 @@ public class GridQueryProcessor extends GridProcessorAdapter {
}
/**
+ * @param sql Query to execute.
+ * @return {@code true} if the given query should be executed with Calcite-based engine.
+ */
+ public static boolean shouldRedirectToCalcite(String sql) {
+ return IS_SELECT_OR_EXPLAIN_PATTERN.matcher(sql).find();
+ }
+
+ /**
* @return Affinity topology version of the current request.
*/
public static AffinityTopologyVersion getRequestAffinityTopologyVersion() {