> On Jan. 26, 2015, 5:51 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java,
> >  line 38
> > <https://reviews.apache.org/r/30051/diff/3/?file=828952#file828952line38>
> >
> >     Please add a negative test case when using multiple partitions.  In 
> > that case, the failure should happen in the planning phase, not execution.

WindowPrel.getPhysicalOperator(PhysicalPlanCreator creator) has the following:
```    
checkState(windows.size() == 1, "Only one window is expected in WindowPrel");
```


- abdelhakim


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30051/#review69617
-----------------------------------------------------------


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
> 
>

Reply via email to