> On Jan. 30, 2015, 1:12 a.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/GenerateTestData.java, > > line 121 > > <https://reviews.apache.org/r/30051/diff/2/?file=826885#file826885line121> > > > > Make the file prefix a command-line argument so that people besides > > yourself can run this.
Argh, I forgot to remove GenerateTestData again! I just used this to generate the data used in the tests, it was never intended to be part of the final code. Sorry about that > On Jan. 30, 2015, 1:12 a.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/GenerateTestData.java, > > line 172 > > <https://reviews.apache.org/r/30051/diff/2/?file=826885#file826885line172> > > > > add > > final Partition partition = partitions[p]; > > and then use partition throughout the loop body. This avoids bounds > > checking and dereferencing on every use. same as above > On Jan. 30, 2015, 1:12 a.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java, > > line 55 > > <https://reviews.apache.org/r/30051/diff/2/?file=826891#file826891line55> > > > > The logger should be private. Yes, I know a lot of them aren't, and > > that's a bug. Good catch. Will fix it asap > On Jan. 30, 2015, 1:12 a.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameTemplate.java, > > line 34 > > <https://reviews.apache.org/r/30051/diff/2/?file=826892#file826892line34> > > > > Can container and batches be final? yes indeed. - abdelhakim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30051/#review70296 ----------------------------------------------------------- On Jan. 28, 2015, 7:50 p.m., abdelhakim deneche wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30051/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2015, 7:50 p.m.) > > > Review request for drill. > > > Bugs: DRILL-1908 > https://issues.apache.org/jira/browse/DRILL-1908 > > > Repository: drill-git > > > Description > ------- > > In order to fix DRILL-1487 a complete rewrite of the > StreamingWindowFrameRecordBatch was needed. This patch adds a new > WindowFrameRecordBatch that correctly handles window functions with or > without order by clauses. > This code still lacks support for frame clauses and may be optimized to > reduce unneeded frame computations. > > > Diffs > ----- > > > common/src/main/java/org/apache/drill/common/logical/data/AbstractBuilder.java > 28424a5 > common/src/main/java/org/apache/drill/common/logical/data/Window.java > 6dba77c > contrib/data/pom.xml 86075f2 > contrib/data/window-test-data/pom.xml PRE-CREATION > exec/java-exec/pom.xml 90734a5 > exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java > 190c13f > exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java > 5288f5d > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/WindowPOP.java > 17738ee > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/OverFinder.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameBatchCreator.java > 9b8929f > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java > 26d23f2 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java > e2c7e9e > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFramer.java > 9588cef > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameBatchCreator.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameTemplate.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFramer.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java > 3b7adca > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/StreamingWindowPrel.java > f1a8bc0 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/StreamingWindowPrule.java > 00c20b2 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WindowPrel.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WindowPrule.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java > 79603eb > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java > f20627d > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java > a9d2ef8 > > exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestWindowFunctions.java > 6eff6db > > Diff: https://reviews.apache.org/r/30051/diff/ > > > Testing > ------- > > Unit tests are available to test window functions in mulitple scenarios: > - b1.p1: single batch with a single partition > - b1.p2: 2 batches, each containing a different parition > - b2.p4: 2 batches and 4 partitions, one partition has rows in both batches > - b3.p2: 3 batches and 2 partitions, one partition includes the whole 2nd > batch and has rows in 3 batches > - b4.p4: 4 batches and 4 partitions, the partitions are arranged to test an > edge case: the 2nd time innerNext() is called, WindowFrameRecordBatch has > enough saved batches to call it's framer.doWork() without the need to call > next(incoming) > > All tests, except the last one, come in 2 variations: with and without "order > by" clause > > > Thanks, > > abdelhakim deneche > >