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 <[email protected]> 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 [email protected].
