Updated Branches: refs/heads/master d405c70df -> ce7cf0d26
Fix ORDER BY and UNION after upgrade to optiq-0.4.9. ORDER BY ... DESC is still broken. Nulls should appear first. Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/2fcff3e7 Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/2fcff3e7 Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/2fcff3e7 Branch: refs/heads/master Commit: 2fcff3e7a161bdfdf61b93208b208d9fd1fba916 Parents: d405c70 Author: Julian Hyde <[email protected]> Authored: Mon Aug 5 14:12:23 2013 -0700 Committer: Julian Hyde <[email protected]> Committed: Mon Aug 5 14:12:23 2013 -0700 ---------------------------------------------------------------------- .../org/apache/drill/optiq/DrillSortRule.java | 5 ++-- .../org/apache/drill/optiq/DrillUnionRel.java | 13 ++++++--- .../org/apache/drill/jdbc/test/JdbcTest.java | 29 ++++++++++++++++---- 3 files changed, 35 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2fcff3e7/sandbox/prototype/sqlparser/src/main/java/org/apache/drill/optiq/DrillSortRule.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/sqlparser/src/main/java/org/apache/drill/optiq/DrillSortRule.java b/sandbox/prototype/sqlparser/src/main/java/org/apache/drill/optiq/DrillSortRule.java index 469d8bb..0d9852a 100644 --- a/sandbox/prototype/sqlparser/src/main/java/org/apache/drill/optiq/DrillSortRule.java +++ b/sandbox/prototype/sqlparser/src/main/java/org/apache/drill/optiq/DrillSortRule.java @@ -33,10 +33,11 @@ public class DrillSortRule extends RelOptRule { @Override public void onMatch(RelOptRuleCall call) { - final SortRel sort = (SortRel) call.rel(0); + final SortRel sort = call.rel(0); final RelNode input = call.rel(1); final RelTraitSet traits = sort.getTraitSet().plus(DrillRel.CONVENTION); - final RelNode convertedInput = convert(input, traits); + final RelTraitSet inputTraits = input.getTraitSet().plus(DrillRel.CONVENTION); + final RelNode convertedInput = convert(input, inputTraits); call.transformTo(new DrillSortRel(sort.getCluster(), traits, convertedInput, sort.getCollation())); } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2fcff3e7/sandbox/prototype/sqlparser/src/main/java/org/apache/drill/optiq/DrillUnionRel.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/sqlparser/src/main/java/org/apache/drill/optiq/DrillUnionRel.java b/sandbox/prototype/sqlparser/src/main/java/org/apache/drill/optiq/DrillUnionRel.java index e8b35a6..a46816b 100644 --- a/sandbox/prototype/sqlparser/src/main/java/org/apache/drill/optiq/DrillUnionRel.java +++ b/sandbox/prototype/sqlparser/src/main/java/org/apache/drill/optiq/DrillUnionRel.java @@ -22,8 +22,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import net.hydromatic.linq4j.Ord; import org.eigenbase.rel.UnionRelBase; import org.eigenbase.rel.RelNode; -import org.eigenbase.relopt.RelOptCluster; -import org.eigenbase.relopt.RelTraitSet; +import org.eigenbase.relopt.*; import java.util.ArrayList; import java.util.List; @@ -45,6 +44,12 @@ public class DrillUnionRel extends UnionRelBase implements DrillRel { } @Override + public RelOptCost computeSelfCost(RelOptPlanner planner) { + // divide cost by two to ensure cheaper than EnumerableDrillRel + return super.computeSelfCost(planner).multiplyBy(.5); + } + + @Override public int implement(DrillImplementor implementor) { List<Integer> inputIds = new ArrayList<>(); for (Ord<RelNode> input : Ord.zip(inputs)) { @@ -54,8 +59,8 @@ public class DrillUnionRel extends UnionRelBase implements DrillRel { E.g. { op: "union", distinct: true, - inputs: [2, 4] - } + inputs: [2, 4] + } */ final ObjectNode union = implementor.mapper.createObjectNode(); union.put("op", "union"); http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/2fcff3e7/sandbox/prototype/sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java ---------------------------------------------------------------------- diff --git a/sandbox/prototype/sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java b/sandbox/prototype/sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java index 043d4fc..1900dc3 100644 --- a/sandbox/prototype/sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java +++ b/sandbox/prototype/sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java @@ -338,15 +338,15 @@ public class JdbcTest { .planContains("'op':'union','distinct':false"); } - @Test @Ignore + @Test public void testUnion() throws Exception { JdbcAssert.withModel(MODEL, "HR").sql("select deptId from dept\n" + "union\n" + "select deptId from emp") .returnsUnordered("DEPTID=31", "DEPTID=33", "DEPTID=34", "DEPTID=35", "DEPTID=null") .planContains("'op':'union','distinct':true"); } - @Test @Ignore - public void testOrderBy() throws Exception { + @Test + public void testOrderByDescNullsFirst() throws Exception { // desc nulls last JdbcAssert .withModel(MODEL, "HR") @@ -355,6 +355,10 @@ public class JdbcTest { "DEPTID=null; LASTNAME=John\n" + "DEPTID=34; LASTNAME=Robinson\n" + "DEPTID=34; LASTNAME=Smith\n" + "DEPTID=33; LASTNAME=Jones\n" + "DEPTID=33; LASTNAME=Steinberg\n" + "DEPTID=31; LASTNAME=Rafferty\n") .planContains("'op':'order'"); + } + + @Test + public void testOrderByDescNullsLast() throws Exception { // desc nulls first JdbcAssert .withModel(MODEL, "HR") @@ -363,7 +367,12 @@ public class JdbcTest { "DEPTID=34; LASTNAME=Robinson\n" + "DEPTID=34; LASTNAME=Smith\n" + "DEPTID=33; LASTNAME=Jones\n" + "DEPTID=33; LASTNAME=Steinberg\n" + "DEPTID=31; LASTNAME=Rafferty\n" + "DEPTID=null; LASTNAME=John\n") .planContains("'op':'order'"); - // desc is implicitly "nulls first" + } + + @Test @Ignore + public void testOrderByDesc() throws Exception { + // desc is implicitly "nulls first" (i.e. null sorted as +inf) + // Current behavior is to sort nulls last. This is wrong. JdbcAssert .withModel(MODEL, "HR") .sql("select * from emp order by deptId desc") @@ -371,13 +380,21 @@ public class JdbcTest { "DEPTID=null; LASTNAME=John\n" + "DEPTID=34; LASTNAME=Robinson\n" + "DEPTID=34; LASTNAME=Smith\n" + "DEPTID=33; LASTNAME=Jones\n" + "DEPTID=33; LASTNAME=Steinberg\n" + "DEPTID=31; LASTNAME=Rafferty\n") .planContains("'op':'order'"); + } + + @Test + public void testOrderBy() throws Exception { // no sort order specified is implicitly "asc", and asc is "nulls last" JdbcAssert .withModel(MODEL, "HR") .sql("select * from emp order by deptId") .returns( - "DEPTID=null; LASTNAME=John\n" + "DEPTID=31; LASTNAME=Rafferty\n" + "DEPTID=33; LASTNAME=Jones\n" - + "DEPTID=33; LASTNAME=Steinberg\n" + "DEPTID=34; LASTNAME=Robinson\n" + "DEPTID=34; LASTNAME=Smith\n") + "DEPTID=31; LASTNAME=Rafferty\n" + + "DEPTID=33; LASTNAME=Jones\n" + + "DEPTID=33; LASTNAME=Steinberg\n" + + "DEPTID=34; LASTNAME=Robinson\n" + + "DEPTID=34; LASTNAME=Smith\n" + + "DEPTID=null; LASTNAME=John\n") .planContains("'op':'order'"); } }
