You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Sean Hsuan-Yi Chu <hs...@usc.edu> on 2015/06/11 19:51:44 UTC
Review Request 35365: DRILL-3182, DRILL-3188, DRILL-3195
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35365/
-----------------------------------------------------------
Review request for drill, Aman Sinha and Jinfeng Ni.
Bugs: DRILL-3182, DRILL-3188 and DRILL-3195
https://issues.apache.org/jira/browse/DRILL-3182
https://issues.apache.org/jira/browse/DRILL-3188
https://issues.apache.org/jira/browse/DRILL-3195
Repository: drill-git
Description
-------
Window function with DISTINCT qualifier is disabled; Disable unsupported window frames; Window functions NTILE, LAG, LEAD, FIRST_VALUE, LAST_VALUE are disabled
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java b92de3b
exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java fc75d73
Diff: https://reviews.apache.org/r/35365/diff/
Testing
-------
Passed Unit tests
The rest is on the way
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 35365: DRILL-3182, DRILL-3188, DRILL-3195
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35365/#review88005
-----------------------------------------------------------
Ship it!
Ship It!
- Aman Sinha
On June 16, 2015, 12:04 a.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35365/
> -----------------------------------------------------------
>
> (Updated June 16, 2015, 12:04 a.m.)
>
>
> Review request for drill, Aman Sinha and Jinfeng Ni.
>
>
> Bugs: DRILL-3182, DRILL-3188 and DRILL-3195
> https://issues.apache.org/jira/browse/DRILL-3182
> https://issues.apache.org/jira/browse/DRILL-3188
> https://issues.apache.org/jira/browse/DRILL-3195
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Window function with DISTINCT qualifier is disabled; Disable unsupported window frames; Window functions NTILE, LAG, LEAD, FIRST_VALUE, LAST_VALUE are disabled
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java f3c54cc
> exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java fc75d73
>
> Diff: https://reviews.apache.org/r/35365/diff/
>
>
> Testing
> -------
>
> All the required tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 35365: DRILL-3182, DRILL-3188, DRILL-3195
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35365/
-----------------------------------------------------------
(Updated June 16, 2015, 12:04 a.m.)
Review request for drill, Aman Sinha and Jinfeng Ni.
Changes
-------
Update the message
Bugs: DRILL-3182, DRILL-3188 and DRILL-3195
https://issues.apache.org/jira/browse/DRILL-3182
https://issues.apache.org/jira/browse/DRILL-3188
https://issues.apache.org/jira/browse/DRILL-3195
Repository: drill-git
Description
-------
Window function with DISTINCT qualifier is disabled; Disable unsupported window frames; Window functions NTILE, LAG, LEAD, FIRST_VALUE, LAST_VALUE are disabled
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java f3c54cc
exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java fc75d73
Diff: https://reviews.apache.org/r/35365/diff/
Testing
-------
All the required tests
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 35365: DRILL-3182, DRILL-3188, DRILL-3195
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35365/
-----------------------------------------------------------
(Updated June 15, 2015, 5:38 p.m.)
Review request for drill, Aman Sinha and Jinfeng Ni.
Changes
-------
New Patch
Bugs: DRILL-3182, DRILL-3188 and DRILL-3195
https://issues.apache.org/jira/browse/DRILL-3182
https://issues.apache.org/jira/browse/DRILL-3188
https://issues.apache.org/jira/browse/DRILL-3195
Repository: drill-git
Description
-------
Window function with DISTINCT qualifier is disabled; Disable unsupported window frames; Window functions NTILE, LAG, LEAD, FIRST_VALUE, LAST_VALUE are disabled
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java f3c54cc
exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java fc75d73
Diff: https://reviews.apache.org/r/35365/diff/
Testing
-------
All the required tests
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 35365: DRILL-3182, DRILL-3188, DRILL-3195
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
> On June 12, 2015, 12:24 a.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java, line 155
> > <https://reviews.apache.org/r/35365/diff/2/?file=983482#file983482line155>
> >
> > we should check for ROWS and RANGE also..
Have modified the if condition
> On June 12, 2015, 12:24 a.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java, line 171
> > <https://reviews.apache.org/r/35365/diff/2/?file=983482#file983482line171>
> >
> > Can we make this message more descriptive ? default frame is generic.
Added the info in:
"When OVER-CLAUSE contains an ORDER BY, " +
"only window frames RANGE UNBOUNDED PRECEDING " +
"and RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW are supported \n" +
"When OVER-CLAUSE does not contain an ORDER BY, " +
"only window frames RANGE BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING " +
"and ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING \n" +
"See Apache Drill JIRA: DRILL-3188");
- Sean Hsuan-Yi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35365/#review87660
-----------------------------------------------------------
On June 15, 2015, 5:38 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35365/
> -----------------------------------------------------------
>
> (Updated June 15, 2015, 5:38 p.m.)
>
>
> Review request for drill, Aman Sinha and Jinfeng Ni.
>
>
> Bugs: DRILL-3182, DRILL-3188 and DRILL-3195
> https://issues.apache.org/jira/browse/DRILL-3182
> https://issues.apache.org/jira/browse/DRILL-3188
> https://issues.apache.org/jira/browse/DRILL-3195
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Window function with DISTINCT qualifier is disabled; Disable unsupported window frames; Window functions NTILE, LAG, LEAD, FIRST_VALUE, LAST_VALUE are disabled
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java f3c54cc
> exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java fc75d73
>
> Diff: https://reviews.apache.org/r/35365/diff/
>
>
> Testing
> -------
>
> All the required tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 35365: DRILL-3182, DRILL-3188, DRILL-3195
Posted by Aman Sinha <as...@maprtech.com>.
> On June 12, 2015, 12:24 a.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java, line 171
> > <https://reviews.apache.org/r/35365/diff/2/?file=983482#file983482line171>
> >
> > Can we make this message more descriptive ? default frame is generic.
>
> Sean Hsuan-Yi Chu wrote:
> Added the info in:
>
> "When OVER-CLAUSE contains an ORDER BY, " +
> "only window frames RANGE UNBOUNDED PRECEDING " +
> "and RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW are supported \n" +
> "When OVER-CLAUSE does not contain an ORDER BY, " +
> "only window frames RANGE BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING " +
> "and ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING \n" +
> "See Apache Drill JIRA: DRILL-3188");
This is probably too descriptive. Let's just say 'This type of window frame is currently not supported' followed by the JIRA that describes what is not supported.
- Aman
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35365/#review87660
-----------------------------------------------------------
On June 15, 2015, 5:38 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35365/
> -----------------------------------------------------------
>
> (Updated June 15, 2015, 5:38 p.m.)
>
>
> Review request for drill, Aman Sinha and Jinfeng Ni.
>
>
> Bugs: DRILL-3182, DRILL-3188 and DRILL-3195
> https://issues.apache.org/jira/browse/DRILL-3182
> https://issues.apache.org/jira/browse/DRILL-3188
> https://issues.apache.org/jira/browse/DRILL-3195
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Window function with DISTINCT qualifier is disabled; Disable unsupported window frames; Window functions NTILE, LAG, LEAD, FIRST_VALUE, LAST_VALUE are disabled
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java f3c54cc
> exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java fc75d73
>
> Diff: https://reviews.apache.org/r/35365/diff/
>
>
> Testing
> -------
>
> All the required tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 35365: DRILL-3182, DRILL-3188, DRILL-3195
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35365/#review87660
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
<https://reviews.apache.org/r/35365/#comment140067>
we should check for ROWS and RANGE also..
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
<https://reviews.apache.org/r/35365/#comment140066>
Can we make this message more descriptive ? default frame is generic.
- Aman Sinha
On June 11, 2015, 11:46 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35365/
> -----------------------------------------------------------
>
> (Updated June 11, 2015, 11:46 p.m.)
>
>
> Review request for drill, Aman Sinha and Jinfeng Ni.
>
>
> Bugs: DRILL-3182, DRILL-3188 and DRILL-3195
> https://issues.apache.org/jira/browse/DRILL-3182
> https://issues.apache.org/jira/browse/DRILL-3188
> https://issues.apache.org/jira/browse/DRILL-3195
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Window function with DISTINCT qualifier is disabled; Disable unsupported window frames; Window functions NTILE, LAG, LEAD, FIRST_VALUE, LAST_VALUE are disabled
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java b92de3b
> exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java fc75d73
>
> Diff: https://reviews.apache.org/r/35365/diff/
>
>
> Testing
> -------
>
> All the required tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 35365: DRILL-3182, DRILL-3188, DRILL-3195
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35365/
-----------------------------------------------------------
(Updated June 11, 2015, 11:46 p.m.)
Review request for drill, Aman Sinha and Jinfeng Ni.
Changes
-------
new patch
Bugs: DRILL-3182, DRILL-3188 and DRILL-3195
https://issues.apache.org/jira/browse/DRILL-3182
https://issues.apache.org/jira/browse/DRILL-3188
https://issues.apache.org/jira/browse/DRILL-3195
Repository: drill-git
Description
-------
Window function with DISTINCT qualifier is disabled; Disable unsupported window frames; Window functions NTILE, LAG, LEAD, FIRST_VALUE, LAST_VALUE are disabled
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java b92de3b
exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java fc75d73
Diff: https://reviews.apache.org/r/35365/diff/
Testing
-------
All the required tests
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 35365: DRILL-3182, DRILL-3188, DRILL-3195
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
> On June 11, 2015, 11:20 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java, line 120
> > <https://reviews.apache.org/r/35365/diff/1/?file=982877#file982877line120>
> >
> > Instead of 'disabled' we should be consistent with the error messages and say 'The window function <name> is not currently supported <rest of the message>'
A new message:
"The window function " + functionName + " is not supported\n"
> On June 11, 2015, 11:20 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java, line 134
> > <https://reviews.apache.org/r/35365/diff/1/?file=982877#file982877line134>
> >
> > Change to 'DISTINCT for window aggregate functions is not currently supported...' (since DISTINCT only applies to window aggregates not to rank, dense_rank etc.)
Update according to the comment
> On June 11, 2015, 11:20 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java, line 158
> > <https://reviews.apache.org/r/35365/diff/1/?file=982877#file982877line158>
> >
> > Instead of using string comparisons, we should use the methods SqlWindow.isCurrentRow(), SqlWindow.isUnboundedPreceding() etc. or the enum Bound.
Update according to the comment
> On June 11, 2015, 11:20 p.m., Aman Sinha wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java, line 105
> > <https://reviews.apache.org/r/35365/diff/1/?file=982878#file982878line105>
> >
> > On the latest master branch, the window.enable option is now set to true, so you can remove these from the unit tests.
Update according to the comment
> On June 11, 2015, 11:20 p.m., Aman Sinha wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java, line 148
> > <https://reviews.apache.org/r/35365/diff/1/?file=982878#file982878line148>
> >
> > For conciseness, it would be better to have a single test which has a query string which takes various types of windows functions that are unsupported (lead, lag etc.) and just runs them one by one. This will reduce the duplication. In any case, in a subsequent release we will be supporting these functions.
Sounds concise. But in these cases, the correct behavior is "seeing an expected exception", which junit would validate this.
For sure, we can use a while loop to wrap try-catch to take over junit's job. But would that presents some confusion ?
- Sean Hsuan-Yi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35365/#review87639
-----------------------------------------------------------
On June 11, 2015, 8:33 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35365/
> -----------------------------------------------------------
>
> (Updated June 11, 2015, 8:33 p.m.)
>
>
> Review request for drill, Aman Sinha and Jinfeng Ni.
>
>
> Bugs: DRILL-3182, DRILL-3188 and DRILL-3195
> https://issues.apache.org/jira/browse/DRILL-3182
> https://issues.apache.org/jira/browse/DRILL-3188
> https://issues.apache.org/jira/browse/DRILL-3195
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Window function with DISTINCT qualifier is disabled; Disable unsupported window frames; Window functions NTILE, LAG, LEAD, FIRST_VALUE, LAST_VALUE are disabled
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java b92de3b
> exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java fc75d73
>
> Diff: https://reviews.apache.org/r/35365/diff/
>
>
> Testing
> -------
>
> All the required tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 35365: DRILL-3182, DRILL-3188, DRILL-3195
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35365/#review87639
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
<https://reviews.apache.org/r/35365/#comment140030>
Instead of 'disabled' we should be consistent with the error messages and say 'The window function <name> is not currently supported <rest of the message>'
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
<https://reviews.apache.org/r/35365/#comment140039>
Change to 'DISTINCT for window aggregate functions is not currently supported...' (since DISTINCT only applies to window aggregates not to rank, dense_rank etc.)
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
<https://reviews.apache.org/r/35365/#comment140048>
Instead of using string comparisons, we should use the methods SqlWindow.isCurrentRow(), SqlWindow.isUnboundedPreceding() etc. or the enum Bound.
exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java
<https://reviews.apache.org/r/35365/#comment140042>
On the latest master branch, the window.enable option is now set to true, so you can remove these from the unit tests.
exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java
<https://reviews.apache.org/r/35365/#comment140045>
For conciseness, it would be better to have a single test which has a query string which takes various types of windows functions that are unsupported (lead, lag etc.) and just runs them one by one. This will reduce the duplication. In any case, in a subsequent release we will be supporting these functions.
- Aman Sinha
On June 11, 2015, 8:33 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35365/
> -----------------------------------------------------------
>
> (Updated June 11, 2015, 8:33 p.m.)
>
>
> Review request for drill, Aman Sinha and Jinfeng Ni.
>
>
> Bugs: DRILL-3182, DRILL-3188 and DRILL-3195
> https://issues.apache.org/jira/browse/DRILL-3182
> https://issues.apache.org/jira/browse/DRILL-3188
> https://issues.apache.org/jira/browse/DRILL-3195
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Window function with DISTINCT qualifier is disabled; Disable unsupported window frames; Window functions NTILE, LAG, LEAD, FIRST_VALUE, LAST_VALUE are disabled
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java b92de3b
> exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java fc75d73
>
> Diff: https://reviews.apache.org/r/35365/diff/
>
>
> Testing
> -------
>
> All the required tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 35365: DRILL-3182, DRILL-3188, DRILL-3195
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35365/
-----------------------------------------------------------
(Updated June 11, 2015, 8:33 p.m.)
Review request for drill, Aman Sinha and Jinfeng Ni.
Changes
-------
Required tests passed
Bugs: DRILL-3182, DRILL-3188 and DRILL-3195
https://issues.apache.org/jira/browse/DRILL-3182
https://issues.apache.org/jira/browse/DRILL-3188
https://issues.apache.org/jira/browse/DRILL-3195
Repository: drill-git
Description
-------
Window function with DISTINCT qualifier is disabled; Disable unsupported window frames; Window functions NTILE, LAG, LEAD, FIRST_VALUE, LAST_VALUE are disabled
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java b92de3b
exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java fc75d73
Diff: https://reviews.apache.org/r/35365/diff/
Testing (updated)
-------
All the required tests
Thanks,
Sean Hsuan-Yi Chu