You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by jinfengni <gi...@git.apache.org> on 2015/09/25 17:40:39 UTC
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
GitHub user jinfengni opened a pull request:
https://github.com/apache/drill/pull/169
DRILL-1457: Push Limit past through UnionExchange.
Aman, could you please review this pull request?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jinfengni/incubator-drill DRILL-1457
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/169.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #169
----
commit 2cf3601d9bd6ba4b338860d7c4d0c3b009253392
Author: Jinfeng Ni <jn...@apache.org>
Date: 2015-09-22T05:42:23Z
DRILL-1457: Push Limit past through UnionExchange.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by julianhyde <gi...@git.apache.org>.
Github user julianhyde commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40447157
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/LimitUnionExchangeTransposeRule.java ---
@@ -0,0 +1,64 @@
+/**
+ * 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.planner.physical;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.drill.exec.planner.logical.RelOptHelper;
+
+import java.math.BigDecimal;
+
+public class LimitUnionExchangeTransposeRule extends Prule{
+ public static final RelOptRule INSTANCE = new LimitUnionExchangeTransposeRule();
+
+ private LimitUnionExchangeTransposeRule() {
+ super(RelOptHelper.some(LimitPrel.class, RelOptHelper.any(UnionExchangePrel.class)), "LimitUnionExchangeTransposeRule");
+ }
+
+ @Override
+ public boolean matches(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+
+ return !limit.isPushDown();
+ }
+
+ @Override
+ public void onMatch(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+ final UnionExchangePrel unionExchangePrel = (UnionExchangePrel) call.rel(1);
+
+ RelNode child = unionExchangePrel.getInput();
+
+ final int offset = limit.getOffset() != null ? Math.max(0, RexLiteral.intValue(limit.getOffset())) : 0;
+ final int fetch = limit.getFetch() != null? Math.max(0, RexLiteral.intValue(limit.getFetch())) : 0;
--- End diff --
0 is a valid limit. I think you need -1 here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by hsuanyi <gi...@git.apache.org>.
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40469361
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/LimitUnionExchangeTransposeRule.java ---
@@ -0,0 +1,64 @@
+/**
+ * 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.planner.physical;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.drill.exec.planner.logical.RelOptHelper;
+
+import java.math.BigDecimal;
+
+public class LimitUnionExchangeTransposeRule extends Prule{
+ public static final RelOptRule INSTANCE = new LimitUnionExchangeTransposeRule();
+
+ private LimitUnionExchangeTransposeRule() {
+ super(RelOptHelper.some(LimitPrel.class, RelOptHelper.any(UnionExchangePrel.class)), "LimitUnionExchangeTransposeRule");
+ }
+
+ @Override
+ public boolean matches(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+
+ return !limit.isPushDown();
+ }
+
+ @Override
+ public void onMatch(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+ final UnionExchangePrel unionExchangePrel = (UnionExchangePrel) call.rel(1);
+
+ RelNode child = unionExchangePrel.getInput();
+
+ final int offset = limit.getOffset() != null ? Math.max(0, RexLiteral.intValue(limit.getOffset())) : 0;
+ final int fetch = limit.getFetch() != null? Math.max(0, RexLiteral.intValue(limit.getFetch())) : 0;
--- End diff --
Cool! I read that one. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by hsuanyi <gi...@git.apache.org>.
Github user hsuanyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40449942
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/LimitUnionExchangeTransposeRule.java ---
@@ -0,0 +1,64 @@
+/**
+ * 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.planner.physical;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.drill.exec.planner.logical.RelOptHelper;
+
+import java.math.BigDecimal;
+
+public class LimitUnionExchangeTransposeRule extends Prule{
+ public static final RelOptRule INSTANCE = new LimitUnionExchangeTransposeRule();
+
+ private LimitUnionExchangeTransposeRule() {
+ super(RelOptHelper.some(LimitPrel.class, RelOptHelper.any(UnionExchangePrel.class)), "LimitUnionExchangeTransposeRule");
+ }
+
+ @Override
+ public boolean matches(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+
+ return !limit.isPushDown();
+ }
+
+ @Override
+ public void onMatch(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+ final UnionExchangePrel unionExchangePrel = (UnionExchangePrel) call.rel(1);
+
+ RelNode child = unionExchangePrel.getInput();
+
+ final int offset = limit.getOffset() != null ? Math.max(0, RexLiteral.intValue(limit.getOffset())) : 0;
+ final int fetch = limit.getFetch() != null? Math.max(0, RexLiteral.intValue(limit.getFetch())) : 0;
--- End diff --
Overall, the approach seems general enough that it is applicable to more than just union-exchange. Can we exploit that ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on the pull request:
https://github.com/apache/drill/pull/169#issuecomment-143375295
Revise the code based on comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40484865
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/LimitUnionExchangeTransposeRule.java ---
@@ -0,0 +1,64 @@
+/**
+ * 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.planner.physical;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.drill.exec.planner.logical.RelOptHelper;
+
+import java.math.BigDecimal;
+
+public class LimitUnionExchangeTransposeRule extends Prule{
+ public static final RelOptRule INSTANCE = new LimitUnionExchangeTransposeRule();
+
+ private LimitUnionExchangeTransposeRule() {
+ super(RelOptHelper.some(LimitPrel.class, RelOptHelper.any(UnionExchangePrel.class)), "LimitUnionExchangeTransposeRule");
+ }
+
+ @Override
+ public boolean matches(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+
+ return !limit.isPushDown();
+ }
+
+ @Override
+ public void onMatch(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+ final UnionExchangePrel unionExchangePrel = (UnionExchangePrel) call.rel(1);
+
+ RelNode child = unionExchangePrel.getInput();
+
+ final int offset = limit.getOffset() != null ? Math.max(0, RexLiteral.intValue(limit.getOffset())) : 0;
+ final int fetch = limit.getFetch() != null? Math.max(0, RexLiteral.intValue(limit.getFetch())) : 0;
--- End diff --
Thanks, Julian!
Good point about the case of fetch == null. When fetch == null, the query should return all the remaining rows starting from offset. In such case, seems it does not make sense to push Limit operator, as the underline operator has to go through all the records.
I modified the rule, so that it will not push limit when fetch == null. I also made change to the row count estimation for the case of fetch == null.
For Sean's comment, this patch is for a special case in Drill's physical planing to handle limit on top of UnionExchange. For other regular relational operator, as Julian pointed, Calcite-831 will have the optimization rules and Drill could re-use them once Calcite-831 is done (Drill-636 was opened for those cases).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40445053
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/limit/TestLimitWithExchanges.java ---
@@ -26,4 +30,34 @@
public void testLimitWithExchanges() throws Exception{
testPhysicalFromFile("limit/limit_exchanges.json");
}
+
+ @Test
+ public void testPushLimitPastUnionExchange() throws Exception {
+ // Push limit past through UnionExchange.
+ final String WORKING_PATH = TestTools.getWorkingPath();
+ final String TEST_RES_PATH = WORKING_PATH + "/src/test/resources";
+
+ try {
+ // case 1. single table query.
+ final String sql = String.format("select * from dfs_test.`%s/tpchmulti/region` limit 1 offset 2", TEST_RES_PATH);
+ test("alter session set `planner.slice_target` = 1");
+ // test(sql);
+
+ // Validate the plan
+ final String[] expectedPlan = {"(?s)Limit.*UnionExchange.*Limit.*Scan"};
+ final String[] excludedPatterns = {};
+ PlanTestBase.testPlanMatchingPatterns(sql, expectedPlan, excludedPatterns);
+
+ // case 2: join query.
+ final String sql2 = String.format("select * from dfs_test.`%s/tpchmulti/region` r, dfs_test.`%s/tpchmulti/nation` n where r.r_regionkey = n.n_regionkey limit 1 offset 2", TEST_RES_PATH, TEST_RES_PATH );
+ // Validate the plan
+ final String[] expectedPlan2 = {"(?s)Limit.*UnionExchange.*Limit.*Join"};
+ final String[] excludedPatterns2 = {};
+ PlanTestBase.testPlanMatchingPatterns(sql2, expectedPlan2, excludedPatterns2);
+ } finally {
+ test("alter session set `planner.slice_target` = " + ExecConstants.SLICE_TARGET_OPTION.getDefault().getValue());
+ }
+ // test(sql2);
--- End diff --
Remove extraneous lines..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40444588
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillLimitRelBase.java ---
@@ -36,11 +36,17 @@
public abstract class DrillLimitRelBase extends SingleRel implements DrillRelNode {
protected RexNode offset;
protected RexNode fetch;
+ private boolean pushDown; // whether limit has been push past its child.
--- End diff --
LIMIT is somewhat unique operator in that even when it is pushed past the child, the original LIMIT still remains. Can you add that in the comments because the pushDown flag will be TRUE for the original LIMIT and FALSE for the pushed version.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by julianhyde <gi...@git.apache.org>.
Github user julianhyde commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40446899
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillLimitRelBase.java ---
@@ -36,11 +36,17 @@
public abstract class DrillLimitRelBase extends SingleRel implements DrillRelNode {
protected RexNode offset;
protected RexNode fetch;
+ private boolean pushDown; // whether limit has been push past its child.
--- End diff --
Actually sort and aggregate have some of this behavior too. You can do a partial sort or partial aggregate in the children but then you need to combine.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by julianhyde <gi...@git.apache.org>.
Github user julianhyde commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40465777
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/LimitUnionExchangeTransposeRule.java ---
@@ -0,0 +1,64 @@
+/**
+ * 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.planner.physical;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.drill.exec.planner.logical.RelOptHelper;
+
+import java.math.BigDecimal;
+
+public class LimitUnionExchangeTransposeRule extends Prule{
+ public static final RelOptRule INSTANCE = new LimitUnionExchangeTransposeRule();
+
+ private LimitUnionExchangeTransposeRule() {
+ super(RelOptHelper.some(LimitPrel.class, RelOptHelper.any(UnionExchangePrel.class)), "LimitUnionExchangeTransposeRule");
+ }
+
+ @Override
+ public boolean matches(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+
+ return !limit.isPushDown();
+ }
+
+ @Override
+ public void onMatch(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+ final UnionExchangePrel unionExchangePrel = (UnionExchangePrel) call.rel(1);
+
+ RelNode child = unionExchangePrel.getInput();
+
+ final int offset = limit.getOffset() != null ? Math.max(0, RexLiteral.intValue(limit.getOffset())) : 0;
+ final int fetch = limit.getFetch() != null? Math.max(0, RexLiteral.intValue(limit.getFetch())) : 0;
--- End diff --
@hsuanyi There are several changes going into Calcite for optimizing limits. See https://issues.apache.org/jira/browse/CALCITE-831 and re-use those rules in drill if applicable.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40484389
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillLimitRelBase.java ---
@@ -36,11 +36,17 @@
public abstract class DrillLimitRelBase extends SingleRel implements DrillRelNode {
protected RexNode offset;
protected RexNode fetch;
+ private boolean pushDown; // whether limit has been push past its child.
--- End diff --
I add the comment as Aman suggested.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40484512
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/limit/TestLimitWithExchanges.java ---
@@ -26,4 +30,34 @@
public void testLimitWithExchanges() throws Exception{
testPhysicalFromFile("limit/limit_exchanges.json");
}
+
+ @Test
+ public void testPushLimitPastUnionExchange() throws Exception {
--- End diff --
I added more unit tests with data validation. The data validation is only to verify row count, but not the record. That's because depending on which scan minor fragment first send the data, the final result could be changing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40484523
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/limit/TestLimitWithExchanges.java ---
@@ -26,4 +30,34 @@
public void testLimitWithExchanges() throws Exception{
testPhysicalFromFile("limit/limit_exchanges.json");
}
+
+ @Test
+ public void testPushLimitPastUnionExchange() throws Exception {
+ // Push limit past through UnionExchange.
+ final String WORKING_PATH = TestTools.getWorkingPath();
+ final String TEST_RES_PATH = WORKING_PATH + "/src/test/resources";
+
+ try {
+ // case 1. single table query.
+ final String sql = String.format("select * from dfs_test.`%s/tpchmulti/region` limit 1 offset 2", TEST_RES_PATH);
+ test("alter session set `planner.slice_target` = 1");
+ // test(sql);
+
+ // Validate the plan
+ final String[] expectedPlan = {"(?s)Limit.*UnionExchange.*Limit.*Scan"};
+ final String[] excludedPatterns = {};
+ PlanTestBase.testPlanMatchingPatterns(sql, expectedPlan, excludedPatterns);
+
+ // case 2: join query.
+ final String sql2 = String.format("select * from dfs_test.`%s/tpchmulti/region` r, dfs_test.`%s/tpchmulti/nation` n where r.r_regionkey = n.n_regionkey limit 1 offset 2", TEST_RES_PATH, TEST_RES_PATH );
+ // Validate the plan
+ final String[] expectedPlan2 = {"(?s)Limit.*UnionExchange.*Limit.*Join"};
+ final String[] excludedPatterns2 = {};
+ PlanTestBase.testPlanMatchingPatterns(sql2, expectedPlan2, excludedPatterns2);
+ } finally {
+ test("alter session set `planner.slice_target` = " + ExecConstants.SLICE_TARGET_OPTION.getDefault().getValue());
+ }
+ // test(sql2);
--- End diff --
Removed that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40444952
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/limit/TestLimitWithExchanges.java ---
@@ -26,4 +30,34 @@
public void testLimitWithExchanges() throws Exception{
testPhysicalFromFile("limit/limit_exchanges.json");
}
+
+ @Test
+ public void testPushLimitPastUnionExchange() throws Exception {
--- End diff --
I think we should add a simple test with data validation for the LIMIT pushdown. Also include OFFSET + LIMIT.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on the pull request:
https://github.com/apache/drill/pull/169#issuecomment-143380757
+1 LGTM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/169#discussion_r40444664
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/LimitUnionExchangeTransposeRule.java ---
@@ -0,0 +1,64 @@
+/**
+ * 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.planner.physical;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.drill.exec.planner.logical.RelOptHelper;
+
+import java.math.BigDecimal;
+
+public class LimitUnionExchangeTransposeRule extends Prule{
+ public static final RelOptRule INSTANCE = new LimitUnionExchangeTransposeRule();
+
+ private LimitUnionExchangeTransposeRule() {
+ super(RelOptHelper.some(LimitPrel.class, RelOptHelper.any(UnionExchangePrel.class)), "LimitUnionExchangeTransposeRule");
+ }
+
+ @Override
+ public boolean matches(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+
+ return !limit.isPushDown();
+ }
+
+ @Override
+ public void onMatch(RelOptRuleCall call) {
+ final LimitPrel limit = (LimitPrel) call.rel(0);
+ final UnionExchangePrel unionExchangePrel = (UnionExchangePrel) call.rel(1);
+
+ RelNode child = unionExchangePrel.getInput();
+
+ final int offset = limit.getOffset() != null ? Math.max(0, RexLiteral.intValue(limit.getOffset())) : 0;
+ final int fetch = limit.getFetch() != null? Math.max(0, RexLiteral.intValue(limit.getFetch())) : 0;
+
+ // child Limit uses conservative approach: use offset 0 and fetch = parent limit offset + parent limit fetch.
+ final RexNode childFetch = limit.getCluster().getRexBuilder().makeExactLiteral(BigDecimal.valueOf(offset + fetch));
+
+ // RexNode newFetch = limit.getCluster().getRexBuilder().makeLiteral()
--- End diff --
Remove this commented line...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-1457: Push Limit past through UnionExcha...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/169
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---