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 <kor...@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() {