----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14027/#review25997 -----------------------------------------------------------
DrillLimitRule currently looks for an EnumerableLimitRel. EnumerableLimitRel is different physical implementation of limit, whereas I think DrillLimitRule should look for a logical limit -- i.e. a SortRel that has fetch or offset set. If it finds a SortRel with a fetch or limit, create a DrillLimitRel with that fetch and limit, and on top of a SortRel without any fetch or limit. If that SortRel is trivial (i.e. no sort fields) omit it. EnumerableLimitRule does something very similar -- you could use that code as a starting point. Comments would be extremely helpful. E.g. explain meaning of first and last. What do null values mean? Is the range inclusive or exclusive? It would simplify things if you ensure that DrillSortRel's fetch and offset fields are always null. DrillSortRule should only fire on a SortRel that has null fetch and offset. Then you can remove the call to DrillLimitRel.implementLimit from DrillSortRel. (In future you could make the drill's sort operator more efficient by applying a row limit during the sort. But let's code for the operator as it is today.) - Julian Hyde On Sept. 8, 2013, 5:37 p.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14027/ > ----------------------------------------------------------- > > (Updated Sept. 8, 2013, 5:37 p.m.) > > > Review request for drill and Jacques Nadeau. > > > Repository: drill-git > > > Description > ------- > > Adding Limit operator end to end > > > Diffs > ----- > > common/src/main/java/org/apache/drill/common/logical/data/Limit.java > 1774790 > exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java > c116b59 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractPhysicalVisitor.java > c997db4 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/PhysicalVisitor.java > 97e6795 > > 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 > 9984454 > > 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 > 2ef5295 > > 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 > sqlparser/pom.xml 84b17a0 > sqlparser/src/main/java/org/apache/drill/optiq/DrillLimitRel.java > PRE-CREATION > sqlparser/src/main/java/org/apache/drill/optiq/DrillLimitRule.java > PRE-CREATION > sqlparser/src/main/java/org/apache/drill/optiq/DrillOptiq.java e687435 > sqlparser/src/main/java/org/apache/drill/optiq/DrillSortRel.java 64995c5 > sqlparser/src/main/java/org/apache/drill/optiq/DrillSortRule.java 0d9852a > > Diff: https://reviews.apache.org/r/14027/diff/ > > > Testing > ------- > > > Thanks, > > Timothy Chen > >
