You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by vo...@apache.org on 2018/05/05 19:07:15 UTC

[drill] 01/06: DRILL-6318: Push down limit past flatten is incorrect

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

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

commit ba5a9215aeb891be938ba9997da8360f19699dc5
Author: Oleg <oz...@solit-clouds.ru>
AuthorDate: Tue Apr 10 11:26:25 2018 +0300

    DRILL-6318: Push down limit past flatten is incorrect
    
    closes #1204
---
 .../exec/planner/logical/DrillPushLimitToScanRule.java      | 13 ++++---------
 .../src/test/java/org/apache/drill/TestBugFixes.java        | 13 +++++++++++++
 .../exec/physical/impl/flatten/TestFlattenPlanning.java     |  5 +++--
 exec/java-exec/src/test/resources/jsoninput/bug6318.json    | 12 ++++++++++++
 4 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushLimitToScanRule.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushLimitToScanRule.java
index 2d33d38..79ba9b0 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushLimitToScanRule.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushLimitToScanRule.java
@@ -67,9 +67,11 @@ public abstract class DrillPushLimitToScanRule extends RelOptRule {
       // mess up the schema since Convert_FromJson() is different from other regular functions in that it only knows
       // the output schema after evaluation is performed. When input has 0 row, Drill essentially does not have a way
       // to know the output type.
+      // Cannot pushdown limit and offset in to flatten as long as we don't know data distribution in flattened field
       if (!limitRel.isPushDown() && (limitRel.getFetch() != null)
           && (!DrillRelOptUtil.isLimit0(limitRel.getFetch())
-            || !DrillRelOptUtil.isProjectOutputSchemaUnknown(projectRel))) {
+            || !DrillRelOptUtil.isProjectOutputSchemaUnknown(projectRel))
+          && !DrillRelOptUtil.isProjectOutputRowcountUnknown(projectRel)) {
         return true;
       }
       return false;
@@ -82,14 +84,7 @@ public abstract class DrillPushLimitToScanRule extends RelOptRule {
       RelNode child = projectRel.getInput();
       final RelNode limitUnderProject = limitRel.copy(limitRel.getTraitSet(), ImmutableList.of(child));
       final RelNode newProject = projectRel.copy(projectRel.getTraitSet(), ImmutableList.of(limitUnderProject));
-      if (DrillRelOptUtil.isProjectOutputRowcountUnknown(projectRel)) {
-        //Preserve limit above the project since Flatten can produce more rows. Also mark it so we do not fire the rule again.
-        final RelNode limitAboveProject = new DrillLimitRel(limitRel.getCluster(), limitRel.getTraitSet(), newProject,
-            limitRel.getOffset(), limitRel.getFetch(), true);
-        call.transformTo(limitAboveProject);
-      } else {
-        call.transformTo(newProject);
-      }
+      call.transformTo(newProject);
     }
   };
 
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java b/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java
index 100d194..f22db7b 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java
@@ -22,6 +22,7 @@ import org.apache.drill.categories.UnlikelyTest;
 import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.exec.planner.physical.PlannerSettings;
 import org.apache.drill.test.BaseTestQuery;
+import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Ignore;
 import org.junit.Test;
@@ -300,4 +301,16 @@ public class TestBugFixes extends BaseTestQuery {
       test("ALTER SESSION RESET `planner.slice_target`");
     }
   }
+
+  @Test
+  public void testDRILL6318() throws Exception {
+    int rows = testSql("SELECT FLATTEN(data) AS d FROM cp.`jsoninput/bug6318.json`");
+    Assert.assertEquals(11, rows);
+
+    rows = testSql("SELECT FLATTEN(data) AS d FROM cp.`jsoninput/bug6318.json` LIMIT 3");
+    Assert.assertEquals(3, rows);
+
+    rows = testSql("SELECT FLATTEN(data) AS d FROM cp.`jsoninput/bug6318.json` LIMIT 3 OFFSET 5");
+    Assert.assertEquals(3, rows);
+  }
 }
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/flatten/TestFlattenPlanning.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/flatten/TestFlattenPlanning.java
index 0e2d92c..9731aa2 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/flatten/TestFlattenPlanning.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/flatten/TestFlattenPlanning.java
@@ -66,8 +66,9 @@ public class TestFlattenPlanning extends PlanTestBase {
   @Test // DRILL-6099 : push limit past flatten(project)
   public void testLimitPushdownPastFlatten() throws Exception {
     final String query = "select rownum, flatten(complex) comp from cp.`store/json/test_flatten_mappify2.json` limit 1";
-    final String[] expectedPatterns = {".*Limit\\(fetch=\\[1\\]\\).*",".*Flatten.*",".*Limit\\(fetch=\\[1\\]\\).*"};
-    final String[] excludedPatterns = null;
+    //DRILL-6318 : limit should not push past flatten(project)
+    final String[] expectedPatterns = {"(?s).*Limit.*Flatten.*Project.*"};
+    final String[] excludedPatterns = {"(?s).*Limit.*Flatten.*Limit.*"};
     PlanTestBase.testPlanMatchingPatterns(query, expectedPatterns, excludedPatterns);
   }
 
diff --git a/exec/java-exec/src/test/resources/jsoninput/bug6318.json b/exec/java-exec/src/test/resources/jsoninput/bug6318.json
new file mode 100644
index 0000000..1fdef8e
--- /dev/null
+++ b/exec/java-exec/src/test/resources/jsoninput/bug6318.json
@@ -0,0 +1,12 @@
+[
+	{ "name": "Helpless Templer", "data": [] },
+	{ "name": "Humble Grandma", "data": ["Honored Boy Scout", "Yawning Wolf"] },
+	{ "name": "Slow Stinger", "data": [] }, 
+	{ "name": "Slow Salesman", "data": ["Closed Queen", "Innocent Volunteer", "Junior Wing", "Lame Mantis", "Old Master", "Numb Pawn"] },
+	{ "name": "Mellow Tinkerbell", "data": [] },
+	{ "name": "Digital Mercury", "data": ["Hollow Guardian", "Twin Hurricane"] },
+	{ "name": "Last Beehive", "data": [] },
+	{ "name": "Infamous Balboa", "data": ["Helpless Avalange"] },
+	{ "name": "Cold Nurse", "data": [] },
+	{ "name": "Major Pawn", "data": [] }
+]
\ No newline at end of file

-- 
To stop receiving notification emails like this one, please contact
volodymyr@apache.org.