> On Sept. 18, 2013, 6:39 p.m., Jacques Nadeau wrote: > > sqlparser/src/main/java/org/apache/drill/optiq/DrillSortRule.java, line 38 > > <https://reviews.apache.org/r/14027/diff/2/?file=350260#file350260line38> > > > > This seems weird/wrong. Maybe it is just the comment. Should the > > comment read: // this rule only supports conversion of sorts that don't > > have offset and fetch. > > > > I guess the part that is weird is rules should be independent. And > > there is no guarantee of what order they are applied in so referring to > > another rule in a rule seems wrong. Am I missing something here?
Jacques, yes you're missing something. It's simpler if a rule just does a small amount of work, and relies on work done by other rules. It would be wrong if an Optiq rule called another rule explicitly (and Optiq doesn't allow that). Rules are independent in the same way that targets in a make file are independent; they may rely on output from previous targets. - Julian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14027/#review26231 ----------------------------------------------------------- On Sept. 10, 2013, 8:29 a.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14027/ > ----------------------------------------------------------- > > (Updated Sept. 10, 2013, 8:29 a.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 > exec/ref/src/main/java/org/apache/drill/exec/ref/rops/LimitROP.java 4a29f94 > 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 > sqlparser/src/test/java/org/apache/drill/jdbc/test/FullEngineTest.java > a1a6cf2 > sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java 37e81b7 > > Diff: https://reviews.apache.org/r/14027/diff/ > > > Testing > ------- > > > Thanks, > > Timothy Chen > >
