Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Drill >> mail # dev >> Review Request 14027: Adding Limit operator end to end


Copy link to this message
-
Re: Review Request 14027: Adding Limit operator end to end


> On Sept. 27, 2013, 4:57 p.m., Julian Hyde wrote:
> > sqlparser/src/main/java/org/apache/drill/optiq/DrillLimitRule.java, line 28
> > <https://reviews.apache.org/r/14027/diff/2/?file=350257#file350257line28>
> >
> >     This is wrong. You are creating a sort with no collations, no fetch, no offset. A completely useless rel!
> >    
> >     You need to create a sort with the original collations, but no fetch or offset. The new limit rel is doing the fetch and offset. The sort will do the collations.

So I tried changing different things but not sure why in the end I don't actually have sortRel in the tree.

Here is one of the trials I tried:

-    final RelTraitSet traits = sort.getTraitSet().plus(DrillRel.CONVENTION);
+    final RelTraitSet traits = sort.getTraitSet();
     RelNode input = sort.getChild();
     if (!sort.getCollation().getFieldCollations().isEmpty()) {
       input = sort.copy(
-          sort.getTraitSet().replace(RelCollationImpl.EMPTY),
-          input,
-          RelCollationImpl.EMPTY,
+          sort.getTraitSet(),
+          sort,
+          sort.getCollation(),
           null,
           null);
     }
-    RelNode x = convert(
-        input,
-        input.getTraitSet().replace(DrillRel.CONVENTION));
-    call.transformTo(new DrillLimitRel(sort.getCluster(), traits, x, sort.offset, sort.fetch));
+    //RelNode x = convert(
+    //    input,
+    //    input.getTraitSet());
+    call.transformTo(new DrillLimitRel(sort.getCluster(), traits, input, sort.offset, sort.fetch));

Any clue what I'm doing wrong?

One really odd thing I notice is that the OnMatch rule calls ends up adding lots of the same transformTo object (in both working and not working case) in the queue.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14027/#review26433
-----------------------------------------------------------
On Sept. 21, 2013, 5:56 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14027/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2013, 5:56 a.m.)
>
>
> Review request for drill and Jacques Nadeau.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Adding Limit operator end to end
>
>
> Diffs
> -----
>
>   LICENSE 5234110
>   common/src/main/java/org/apache/drill/common/JSONOptions.java 8157dc6
>   common/src/main/java/org/apache/drill/common/logical/data/Constant.java e05ce41
>   common/src/main/java/org/apache/drill/common/logical/data/Limit.java 93eb182
>   common/src/main/java/org/apache/drill/common/logical/data/LogicalOperator.java 9810e75
>   common/src/main/java/org/apache/drill/common/logical/data/LogicalOperatorBase.java 29c994f
>   exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java 5a1fd6e
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractPhysicalVisitor.java 286144b
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/PhysicalVisitor.java a36b65a
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/Limit.java PRE-CREATION
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java 94acc0e
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitBatchCreator.java PRE-CREATION
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java PRE-CREATION
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/StatsCollector.java 0368d0c
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/limit/TestSimpleLimit.java PRE-CREATION
>   exec/java-exec/src/test/resources/limit/test1.json PRE-CREATION
>   exec/java-exec/src/test/resources/limit/test2.json PRE-CREATION
>   exec/java-exec/src/test/resources/limit/test3.json PRE-CREATION