This is an automated email from the ASF dual-hosted git repository.
cgivre pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git
The following commit(s) were added to refs/heads/master by this push:
new fa2cb0f DRILL-8090: LIMIT clause is pushed down to an invalid
OFFSET-FETCH clause for MS SQL Server (#2420)
fa2cb0f is described below
commit fa2cb0f4937c0d8e797a675d8d6c13c316e48d4c
Author: Volodymyr Vysotskyi <[email protected]>
AuthorDate: Tue Jan 4 00:34:31 2022 +0200
DRILL-8090: LIMIT clause is pushed down to an invalid OFFSET-FETCH clause
for MS SQL Server (#2420)
* DRILL-8090: LIMIT clause is pushed down to an invalid OFFSET-FETCH clause
for MS SQL Server
* DRILL-8090: Changes after code review
---
.../drill/exec/store/jdbc/DrillJdbcConvention.java | 44 +++++++++++-------
.../drill/exec/store/jdbc/rules/JdbcLimitRule.java | 50 ++++++++++++++++++++
.../drill/exec/store/jdbc/rules/JdbcSortRule.java | 49 ++++++++++++++++++++
.../store/phoenix/rules/PhoenixConvention.java | 41 +++++++++-------
.../PhoenixIntermediatePrelConverterRule.java | 54 ++++++++++------------
.../store/enumerable/plan/DrillJdbcRuleBase.java | 2 +-
pom.xml | 2 +-
7 files changed, 177 insertions(+), 65 deletions(-)
diff --git
a/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/DrillJdbcConvention.java
b/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/DrillJdbcConvention.java
index 666502a..c469502 100644
---
a/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/DrillJdbcConvention.java
+++
b/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/DrillJdbcConvention.java
@@ -17,6 +17,7 @@
*/
package org.apache.drill.exec.store.jdbc;
+import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
@@ -25,18 +26,21 @@ import org.apache.calcite.adapter.jdbc.JdbcConvention;
import org.apache.calcite.adapter.jdbc.JdbcRules;
import org.apache.calcite.adapter.jdbc.JdbcRules.JdbcFilterRule;
import org.apache.calcite.adapter.jdbc.JdbcRules.JdbcProjectRule;
-import org.apache.calcite.adapter.jdbc.JdbcRules.JdbcSortRule;
import org.apache.calcite.adapter.jdbc.JdbcToEnumerableConverterRule;
import org.apache.calcite.linq4j.tree.ConstantUntypedNull;
import org.apache.calcite.plan.Convention;
import org.apache.calcite.plan.RelOptPlanner;
import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelTrait;
import org.apache.calcite.sql.SqlDialect;
import org.apache.drill.exec.planner.RuleInstance;
import org.apache.drill.exec.planner.logical.DrillRel;
import org.apache.drill.exec.planner.logical.DrillRelFactories;
+import org.apache.drill.exec.planner.physical.Prel;
import org.apache.drill.exec.store.enumerable.plan.DrillJdbcRuleBase;
import org.apache.drill.exec.store.enumerable.plan.VertexDrelConverterRule;
+import org.apache.drill.exec.store.jdbc.rules.JdbcLimitRule;
+import org.apache.drill.exec.store.jdbc.rules.JdbcSortRule;
import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableSet;
/**
@@ -48,7 +52,7 @@ public class DrillJdbcConvention extends JdbcConvention {
* Unwanted Calcite's JdbcRules are filtered out using this set
*/
private static final Set<Class<? extends RelOptRule>> EXCLUDED_CALCITE_RULES
= ImmutableSet.of(
- JdbcToEnumerableConverterRule.class, JdbcFilterRule.class,
JdbcProjectRule.class, JdbcSortRule.class);
+ JdbcToEnumerableConverterRule.class, JdbcFilterRule.class,
JdbcProjectRule.class, JdbcRules.JdbcSortRule.class);
private final ImmutableSet<RelOptRule> rules;
private final JdbcStoragePlugin plugin;
@@ -59,21 +63,27 @@ public class DrillJdbcConvention extends JdbcConvention {
List<RelOptRule> calciteJdbcRules = JdbcRules.rules(this,
DrillRelFactories.LOGICAL_BUILDER).stream()
.filter(rule -> !EXCLUDED_CALCITE_RULES.contains(rule.getClass()))
.collect(Collectors.toList());
- this.rules = ImmutableSet.<RelOptRule>builder()
- .addAll(calciteJdbcRules)
- .add(new JdbcIntermediatePrelConverterRule(this))
- .add(new VertexDrelConverterRule(this))
- .add(new DrillJdbcRuleBase.DrillJdbcProjectRule(Convention.NONE, this))
- .add(new
DrillJdbcRuleBase.DrillJdbcProjectRule(DrillRel.DRILL_LOGICAL, this))
- .add(new DrillJdbcRuleBase.DrillJdbcFilterRule(Convention.NONE, this))
- .add(new DrillJdbcRuleBase.DrillJdbcFilterRule(DrillRel.DRILL_LOGICAL,
this))
- .add(new DrillJdbcRuleBase.DrillJdbcSortRule(Convention.NONE, this))
- .add(new DrillJdbcRuleBase.DrillJdbcSortRule(DrillRel.DRILL_LOGICAL,
this))
- .add(new DrillJdbcRuleBase.DrillJdbcLimitRule(Convention.NONE, this))
- .add(new DrillJdbcRuleBase.DrillJdbcLimitRule(DrillRel.DRILL_LOGICAL,
this))
- .add(RuleInstance.FILTER_SET_OP_TRANSPOSE_RULE)
- .add(RuleInstance.PROJECT_REMOVE_RULE)
- .build();
+
+ List<RelTrait> inputTraits = Arrays.asList(
+ Convention.NONE,
+ DrillRel.DRILL_LOGICAL,
+ Prel.DRILL_PHYSICAL);
+
+ ImmutableSet.Builder<RelOptRule> builder =
ImmutableSet.<RelOptRule>builder()
+ .addAll(calciteJdbcRules)
+ .add(new JdbcIntermediatePrelConverterRule(this))
+ .add(new VertexDrelConverterRule(this))
+ .add(RuleInstance.FILTER_SET_OP_TRANSPOSE_RULE)
+ .add(RuleInstance.PROJECT_REMOVE_RULE);
+ for (RelTrait inputTrait : inputTraits) {
+ builder
+ .add(new DrillJdbcRuleBase.DrillJdbcProjectRule(inputTrait, this))
+ .add(new DrillJdbcRuleBase.DrillJdbcFilterRule(inputTrait, this))
+ .add(new JdbcSortRule(inputTrait, this))
+ .add(new JdbcLimitRule(inputTrait, this));
+ }
+
+ this.rules = builder.build();
}
@Override
diff --git
a/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/rules/JdbcLimitRule.java
b/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/rules/JdbcLimitRule.java
new file mode 100644
index 0000000..dc44c72
--- /dev/null
+++
b/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/rules/JdbcLimitRule.java
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.jdbc.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelTrait;
+import org.apache.calcite.rel.RelCollations;
+import org.apache.calcite.sql.dialect.MssqlSqlDialect;
+import org.apache.drill.exec.planner.common.DrillLimitRelBase;
+import org.apache.drill.exec.store.enumerable.plan.DrillJdbcRuleBase;
+import org.apache.drill.exec.store.jdbc.DrillJdbcConvention;
+
+public class JdbcLimitRule extends DrillJdbcRuleBase.DrillJdbcLimitRule {
+ private final DrillJdbcConvention convention;
+
+ public JdbcLimitRule(RelTrait in, DrillJdbcConvention out) {
+ super(in, out);
+ this.convention = out;
+ }
+
+ @Override
+ public boolean matches(RelOptRuleCall call) {
+ DrillLimitRelBase limit = call.rel(0);
+ if (super.matches(call)) {
+ // MS SQL doesn't support either OFFSET or FETCH without ORDER BY.
+ // But for the case of FETCH without OFFSET, Calcite generates TOP N
+ // instead of FETCH, and it is supported by MS SQL.
+ // So do not push down only the limit with both OFFSET and FETCH but
without ORDER BY.
+ return limit.getOffset() == null
+ || !limit.getTraitSet().contains(RelCollations.EMPTY)
+ || !(convention.getPlugin().getDialect() instanceof MssqlSqlDialect);
+ }
+ return false;
+ }
+}
diff --git
a/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/rules/JdbcSortRule.java
b/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/rules/JdbcSortRule.java
new file mode 100644
index 0000000..4b52306
--- /dev/null
+++
b/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/rules/JdbcSortRule.java
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.jdbc.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelTrait;
+import org.apache.calcite.rel.core.Sort;
+import org.apache.calcite.sql.dialect.MssqlSqlDialect;
+import org.apache.drill.exec.store.enumerable.plan.DrillJdbcRuleBase;
+import org.apache.drill.exec.store.jdbc.DrillJdbcConvention;
+
+public class JdbcSortRule extends DrillJdbcRuleBase.DrillJdbcSortRule {
+ private final DrillJdbcConvention convention;
+
+ public JdbcSortRule(RelTrait in, DrillJdbcConvention out) {
+ super(in, out);
+ this.convention = out;
+ }
+
+ @Override
+ public boolean matches(RelOptRuleCall call) {
+ Sort sort = call.rel(0);
+ if (super.matches(call)) {
+ // MS SQL doesn't support either OFFSET or FETCH without ORDER BY.
+ // But for the case of FETCH without OFFSET, Calcite generates TOP N
+ // instead of FETCH, and it is supported by MS SQL.
+ // So do not push down only the limit with both OFFSET and FETCH but
without ORDER BY.
+ return sort.offset == null
+ || !sort.getCollation().getFieldCollations().isEmpty()
+ || !(convention.getPlugin().getDialect() instanceof MssqlSqlDialect);
+ }
+ return false;
+ }
+}
diff --git
a/contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/rules/PhoenixConvention.java
b/contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/rules/PhoenixConvention.java
index 09ce9be..6d4ff48 100644
---
a/contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/rules/PhoenixConvention.java
+++
b/contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/rules/PhoenixConvention.java
@@ -17,6 +17,7 @@
*/
package org.apache.drill.exec.store.phoenix.rules;
+import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
@@ -32,10 +33,12 @@ import org.apache.calcite.linq4j.tree.ConstantUntypedNull;
import org.apache.calcite.plan.Convention;
import org.apache.calcite.plan.RelOptPlanner;
import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelTrait;
import org.apache.calcite.sql.SqlDialect;
import org.apache.drill.exec.planner.RuleInstance;
import org.apache.drill.exec.planner.logical.DrillRel;
import org.apache.drill.exec.planner.logical.DrillRelFactories;
+import org.apache.drill.exec.planner.physical.Prel;
import org.apache.drill.exec.store.enumerable.plan.DrillJdbcRuleBase;
import org.apache.drill.exec.store.enumerable.plan.VertexDrelConverterRule;
import org.apache.drill.exec.store.phoenix.PhoenixStoragePlugin;
@@ -60,22 +63,28 @@ public class PhoenixConvention extends JdbcConvention {
.rules(this, DrillRelFactories.LOGICAL_BUILDER).stream()
.filter(rule -> !EXCLUDED_CALCITE_RULES.contains(rule.getClass()))
.collect(Collectors.toList());
- this.rules = ImmutableSet.<RelOptRule>builder()
- .addAll(calciteJdbcRules)
- .add(PhoenixIntermediatePrelConverterRule.INSTANCE)
- .add(new VertexDrelConverterRule(this))
- .add(new DrillJdbcRuleBase.DrillJdbcProjectRule(Convention.NONE, this))
- .add(new
DrillJdbcRuleBase.DrillJdbcProjectRule(DrillRel.DRILL_LOGICAL, this))
- .add(new DrillJdbcRuleBase.DrillJdbcFilterRule(Convention.NONE, this))
- .add(new DrillJdbcRuleBase.DrillJdbcFilterRule(DrillRel.DRILL_LOGICAL,
this))
- .add(new DrillJdbcRuleBase.DrillJdbcSortRule(Convention.NONE, this))
- .add(new DrillJdbcRuleBase.DrillJdbcSortRule(DrillRel.DRILL_LOGICAL,
this))
- .add(new DrillJdbcRuleBase.DrillJdbcLimitRule(Convention.NONE, this))
- .add(new DrillJdbcRuleBase.DrillJdbcLimitRule(DrillRel.DRILL_LOGICAL,
this))
- .add(RuleInstance.FILTER_SET_OP_TRANSPOSE_RULE)
- .add(RuleInstance.PROJECT_REMOVE_RULE)
- .add(new PhoenixJoinRule(Convention.NONE, this))
- .build();
+
+ List<RelTrait> inputTraits = Arrays.asList(
+ Convention.NONE,
+ DrillRel.DRILL_LOGICAL,
+ Prel.DRILL_PHYSICAL);
+
+ ImmutableSet.Builder<RelOptRule> builder =
ImmutableSet.<RelOptRule>builder()
+ .addAll(calciteJdbcRules)
+ .add(new PhoenixIntermediatePrelConverterRule(this))
+ .add(new VertexDrelConverterRule(this))
+ .add(RuleInstance.FILTER_SET_OP_TRANSPOSE_RULE)
+ .add(RuleInstance.PROJECT_REMOVE_RULE);
+ for (RelTrait inputTrait : inputTraits) {
+ builder
+ .add(new DrillJdbcRuleBase.DrillJdbcProjectRule(inputTrait, this))
+ .add(new DrillJdbcRuleBase.DrillJdbcFilterRule(inputTrait, this))
+ .add(new DrillJdbcRuleBase.DrillJdbcSortRule(inputTrait, this))
+ .add(new DrillJdbcRuleBase.DrillJdbcLimitRule(inputTrait, this))
+ .add(new PhoenixJoinRule(inputTrait, this));
+ }
+
+ this.rules = builder.build();
}
@Override
diff --git
a/contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/rules/PhoenixIntermediatePrelConverterRule.java
b/contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/rules/PhoenixIntermediatePrelConverterRule.java
index c823c82..c5eaaf1 100644
---
a/contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/rules/PhoenixIntermediatePrelConverterRule.java
+++
b/contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/rules/PhoenixIntermediatePrelConverterRule.java
@@ -17,50 +17,44 @@
*/
package org.apache.drill.exec.store.phoenix.rules;
-import java.util.function.Predicate;
-
-import org.apache.calcite.plan.Convention;
-import org.apache.calcite.plan.ConventionTraitDef;
+import org.apache.calcite.adapter.jdbc.JdbcConvention;
+import org.apache.calcite.plan.RelOptRule;
import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelTrait;
import org.apache.calcite.rel.RelNode;
-import org.apache.calcite.rel.convert.ConverterRule;
import org.apache.drill.exec.planner.logical.DrillRel;
import org.apache.drill.exec.planner.logical.DrillRelFactories;
+import org.apache.drill.exec.planner.logical.RelOptHelper;
import org.apache.drill.exec.planner.physical.Prel;
import org.apache.drill.exec.store.enumerable.plan.VertexDrel;
-final class PhoenixIntermediatePrelConverterRule extends ConverterRule {
+final class PhoenixIntermediatePrelConverterRule extends RelOptRule {
+
+ private final RelTrait inTrait;
+ private final RelTrait outTrait;
- static final PhoenixIntermediatePrelConverterRule INSTANCE = new
PhoenixIntermediatePrelConverterRule();
+ public PhoenixIntermediatePrelConverterRule(JdbcConvention jdbcConvention) {
+ super(
+ RelOptHelper.some(VertexDrel.class, DrillRel.DRILL_LOGICAL,
+ RelOptHelper.any(RelNode.class, jdbcConvention)),
+ DrillRelFactories.LOGICAL_BUILDER, "Phoenix_PREL_Converter" +
jdbcConvention);
- private PhoenixIntermediatePrelConverterRule() {
- super(VertexDrel.class, (Predicate<RelNode>) input -> true,
- DrillRel.DRILL_LOGICAL, Prel.DRILL_PHYSICAL,
DrillRelFactories.LOGICAL_BUILDER, "Phoenix_PREL_Converter");
+ this.inTrait = DrillRel.DRILL_LOGICAL;
+ this.outTrait = Prel.DRILL_PHYSICAL;
}
@Override
- public RelNode convert(RelNode in) {
- return new PhoenixIntermediatePrel(
- in.getCluster(),
- in.getTraitSet().replace(getOutTrait()),
- in.getInput(0));
+ public void onMatch(RelOptRuleCall call) {
+ VertexDrel in = call.rel(0);
+ RelNode intermediatePrel = new PhoenixIntermediatePrel(
+ in.getCluster(),
+ in.getTraitSet().replace(outTrait),
+ in.getInput(0));
+ call.transformTo(intermediatePrel);
}
@Override
- public void onMatch(RelOptRuleCall call) {
- RelNode rel = call.rel(0);
- if (rel.getTraitSet().contains(getInTrait())) {
- Convention c =
rel.getInput(0).getTraitSet().getTrait(ConventionTraitDef.INSTANCE);
- /*
- * only accept the PhoenixConvention
- * need to avoid the NPE or ClassCastException
- */
- if (c != null && c instanceof PhoenixConvention) {
- final RelNode converted = convert(rel);
- if (converted != null) {
- call.transformTo(converted);
- }
- }
- }
+ public boolean matches(RelOptRuleCall call) {
+ return super.matches(call) && call.rel(0).getTraitSet().contains(inTrait);
}
}
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/enumerable/plan/DrillJdbcRuleBase.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/enumerable/plan/DrillJdbcRuleBase.java
index 6e8a0f4..1a2722d 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/enumerable/plan/DrillJdbcRuleBase.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/enumerable/plan/DrillJdbcRuleBase.java
@@ -54,7 +54,7 @@ public abstract class DrillJdbcRuleBase extends ConverterRule
{
protected final JdbcConvention out;
- private DrillJdbcRuleBase(Class<? extends RelNode> clazz, RelTrait in,
JdbcConvention out, String description) {
+ protected DrillJdbcRuleBase(Class<? extends RelNode> clazz, RelTrait in,
JdbcConvention out, String description) {
super(clazz, (Predicate<RelNode>) input -> true, in, out,
DrillRelFactories.LOGICAL_BUILDER, description);
this.out = out;
}
diff --git a/pom.xml b/pom.xml
index c4f3618..553ffce 100644
--- a/pom.xml
+++ b/pom.xml
@@ -59,7 +59,7 @@
avoid_bad_dependencies plugin found in the file.
-->
<calcite.groupId>com.github.vvysotskyi.drill-calcite</calcite.groupId>
- <calcite.version>1.21.0-drill-r5</calcite.version>
+ <calcite.version>1.21.0-drill-r7</calcite.version>
<avatica.version>1.17.0</avatica.version>
<janino.version>3.0.11</janino.version>
<sqlline.version>1.12.0</sqlline.version>
