drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Aman Sinha" <asi...@maprtech.com>
Subject Re: Review Request 37482: DRILL-3536: Add support for LEAD, LAG, NTILE, FIRST_VALUE and LAST_VALUE window functions
Date Sat, 15 Aug 2015 17:49:30 GMT

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


Sending some initial comments..


exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/DefaultFrameTemplate.java
(line 39)
<https://reviews.apache.org/r/37482/#comment150543>

    It's a little odd to have LAG specific member variable in a class that is shared by all
window functions. A derived class would be better.



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/DefaultFrameTemplate.java
(line 139)
<https://reviews.apache.org/r/37482/#comment150544>

    Similarly for FIRST_VALUE...should this go to a derived class since we don't want to do
this for all window functions ?



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFunction.java
(line 95)
<https://reviews.apache.org/r/37482/#comment150545>

    You might want to change this to WindowAggregate..


- Aman Sinha


On Aug. 14, 2015, 6:40 p.m., abdelhakim deneche wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37482/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 6:40 p.m.)
> 
> 
> Review request for drill and Aman Sinha.
> 
> 
> Bugs: DRILL-3536
>     https://issues.apache.org/jira/browse/DRILL-3536
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> - added support for the new functions in DefaultFrameTemplate
> - use of an internal batch buffer to store values between batches when computing LAG
> - added new aggregate function "holdLast" to store intermediate values between batches
when computing FIRST_VALUE
> - added unit tests for the new functions
> - fixed DRILL-3604, 3605 and 3606
> - GenerateTestData is an internal tool used to generate data files and their expected
results for window function unit tests
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/DefaultFrameTemplate.java
535deaa 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/Partition.java
8d6728e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowDataBatch.java
5045cb3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java
9c8cfc0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFramer.java
69866af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFunction.java
PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
04d1231 
>   exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java 9e09106

>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/GenerateTestData.java
PRE-CREATION 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java
553c4e8 
>   exec/java-exec/src/test/resources/window/3604.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/3605.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/3605.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/window/3606.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/3606.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/window/3648.parquet PRE-CREATION 
>   exec/java-exec/src/test/resources/window/3648.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/3648.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/window/b2.p4.ntile.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/window/b4.p4.fval.pby.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/window/b4.p4.lag.oby.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/window/b4.p4.lag.pby.oby.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/window/b4.p4.lead.oby.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/window/b4.p4.lead.pby.oby.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/window/b4.p4.lval.pby.oby.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/window/b4.p4.oby.tsv 528f2f3 
>   exec/java-exec/src/test/resources/window/b4.p4.pby.oby.tsv a5d630b 
>   exec/java-exec/src/test/resources/window/b4.p4.pby.tsv b2bd5e1 
>   exec/java-exec/src/test/resources/window/b4.p4.tsv 1731fe9 
>   exec/java-exec/src/test/resources/window/b4.p4/0.data.json e91a75c 
>   exec/java-exec/src/test/resources/window/b4.p4/1.data.json 52f375b 
>   exec/java-exec/src/test/resources/window/b4.p4/2.data.json 9ecc5ed 
>   exec/java-exec/src/test/resources/window/b4.p4/3.data.json 32d2ad1 
>   exec/java-exec/src/test/resources/window/fewRowsAllData.parquet PRE-CREATION 
>   exec/java-exec/src/test/resources/window/fval.alltypes.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/fval.pby.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/lag.oby.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/lag.pby.oby.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/lead.oby.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/lead.pby.oby.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/lval.alltypes.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/lval.pby.oby.sql PRE-CREATION 
>   exec/java-exec/src/test/resources/window/ntile.sql PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37482/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> abdelhakim deneche
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message