You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Aihua Xu <ax...@cloudera.com> on 2015/05/11 16:24:34 UTC
Review Request 34040: Refactoring Windowing for sum() to pass
WindowFrameDef
instead of two numbers (1 for number of preceding and 1 for number of
following)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34040/
-----------------------------------------------------------
Review request for hive.
Repository: hive-git
Description
-------
This is the first step to add sum() support for "rows between x preceding and y preceding" and "rows between x following and y following" windowing.
This is just a refactoring without affecting the functionality by passing WindowFrameDef instead of passing # of preceding and # of following.
Diffs
-----
ql/src/java/org/apache/hadoop/hive/ql/parse/PTFTranslator.java 00b43c6
ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/BoundaryDef.java f692fa2
ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/WindowFrameDef.java e08bdd5
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java 12a327f
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFFirstValue.java f679387
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFLastValue.java e099154
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFMax.java a153818
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFStreamingEvaluator.java d68c085
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java ffb7093
ql/src/test/org/apache/hadoop/hive/ql/udaf/TestStreamingSum.java a331e66
Diff: https://reviews.apache.org/r/34040/diff/
Testing
-------
Testing has been done and the unit tests failures seem to be unrelated.
https://issues.apache.org/jira/browse/HIVE-10643
Thanks,
Aihua Xu
Re: Review Request 34040: Refactoring Windowing for sum() to pass
WindowFrameDef instead of two numbers (1 for number of preceding and 1 for
number of following)
Posted by Aihua Xu <ax...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34040/
-----------------------------------------------------------
(Updated May 11, 2015, 8:08 p.m.)
Review request for hive.
Repository: hive-git
Description
-------
This is the first step to add sum() support for "rows between x preceding and y preceding" and "rows between x following and y following" windowing.
This is just a refactoring without affecting the functionality by passing WindowFrameDef instead of passing # of preceding and # of following.
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/parse/PTFTranslator.java 00b43c6
ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/BoundaryDef.java f692fa2
ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/WindowFrameDef.java e08bdd5
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java 12a327f
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFFirstValue.java f679387
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFLastValue.java e099154
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFMax.java a153818
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFStreamingEvaluator.java d68c085
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java ffb7093
ql/src/test/org/apache/hadoop/hive/ql/udaf/TestStreamingSum.java a331e66
Diff: https://reviews.apache.org/r/34040/diff/
Testing
-------
Testing has been done and the unit tests failures seem to be unrelated.
https://issues.apache.org/jira/browse/HIVE-10643
Thanks,
Aihua Xu
Re: Review Request 34040: Refactoring Windowing for sum() to pass
WindowFrameDef instead of two numbers (1 for number of preceding and 1 for
number of following)
Posted by Aihua Xu <ax...@cloudera.com>.
> On May 11, 2015, 6:41 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java, line 225
> > <https://reviews.apache.org/r/34040/diff/1/?file=955315#file955315line225>
> >
> > d can't be null at this point. Don't need ternary operator.
fixed.
> On May 11, 2015, 6:41 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java, line 344
> > <https://reviews.apache.org/r/34040/diff/1/?file=955315#file955315line344>
> >
> > d can't be null here.
Fixed.
> On May 11, 2015, 6:41 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java, line 462
> > <https://reviews.apache.org/r/34040/diff/1/?file=955315#file955315line462>
> >
> > d can't be null here.
Fixed.
> On May 11, 2015, 6:41 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/WindowFrameDef.java, line 51
> > <https://reviews.apache.org/r/34040/diff/1/?file=955309#file955309line51>
> >
> > Should this take into account direction information? e.g., for 3 precceding & 2 following, window size = 6, but for 3 preceeding & 2 following, window size = 1, isn't it?
>
> Aihua Xu wrote:
> You mean 3 preceeding & 2 preceding? Yes. It should be, I haven't changed the logic to handle x preceding and x preceding case yet. Just want to make sure it's just like what it is.
I didn't include the fix in here. Will fix in the next task so that all the logic fix will be in the same patch.
- Aihua
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34040/#review83257
-----------------------------------------------------------
On May 11, 2015, 2:24 p.m., Aihua Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34040/
> -----------------------------------------------------------
>
> (Updated May 11, 2015, 2:24 p.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This is the first step to add sum() support for "rows between x preceding and y preceding" and "rows between x following and y following" windowing.
>
> This is just a refactoring without affecting the functionality by passing WindowFrameDef instead of passing # of preceding and # of following.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/PTFTranslator.java 00b43c6
> ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/BoundaryDef.java f692fa2
> ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/WindowFrameDef.java e08bdd5
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java 12a327f
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFFirstValue.java f679387
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFLastValue.java e099154
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFMax.java a153818
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFStreamingEvaluator.java d68c085
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java ffb7093
> ql/src/test/org/apache/hadoop/hive/ql/udaf/TestStreamingSum.java a331e66
>
> Diff: https://reviews.apache.org/r/34040/diff/
>
>
> Testing
> -------
>
> Testing has been done and the unit tests failures seem to be unrelated.
>
> https://issues.apache.org/jira/browse/HIVE-10643
>
>
> Thanks,
>
> Aihua Xu
>
>
Re: Review Request 34040: Refactoring Windowing for sum() to pass
WindowFrameDef instead of two numbers (1 for number of preceding and 1 for
number of following)
Posted by Aihua Xu <ax...@cloudera.com>.
> On May 11, 2015, 6:41 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/WindowFrameDef.java, line 51
> > <https://reviews.apache.org/r/34040/diff/1/?file=955309#file955309line51>
> >
> > Should this take into account direction information? e.g., for 3 precceding & 2 following, window size = 6, but for 3 preceeding & 2 following, window size = 1, isn't it?
You mean 3 preceeding & 2 preceding? Yes. It should be, I haven't changed the logic to handle x preceding and x preceding case yet. Just want to make sure it's just like what it is.
- Aihua
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34040/#review83257
-----------------------------------------------------------
On May 11, 2015, 2:24 p.m., Aihua Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34040/
> -----------------------------------------------------------
>
> (Updated May 11, 2015, 2:24 p.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This is the first step to add sum() support for "rows between x preceding and y preceding" and "rows between x following and y following" windowing.
>
> This is just a refactoring without affecting the functionality by passing WindowFrameDef instead of passing # of preceding and # of following.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/PTFTranslator.java 00b43c6
> ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/BoundaryDef.java f692fa2
> ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/WindowFrameDef.java e08bdd5
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java 12a327f
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFFirstValue.java f679387
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFLastValue.java e099154
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFMax.java a153818
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFStreamingEvaluator.java d68c085
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java ffb7093
> ql/src/test/org/apache/hadoop/hive/ql/udaf/TestStreamingSum.java a331e66
>
> Diff: https://reviews.apache.org/r/34040/diff/
>
>
> Testing
> -------
>
> Testing has been done and the unit tests failures seem to be unrelated.
>
> https://issues.apache.org/jira/browse/HIVE-10643
>
>
> Thanks,
>
> Aihua Xu
>
>
Re: Review Request 34040: Refactoring Windowing for sum() to pass
WindowFrameDef instead of two numbers (1 for number of preceding and 1 for
number of following)
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34040/#review83257
-----------------------------------------------------------
Mostly looks good. Few minor comments.
ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/WindowFrameDef.java
<https://reviews.apache.org/r/34040/#comment134161>
Should this take into account direction information? e.g., for 3 precceding & 2 following, window size = 6, but for 3 preceeding & 2 following, window size = 1, isn't it?
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java
<https://reviews.apache.org/r/34040/#comment134164>
d can't be null at this point. Don't need ternary operator.
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java
<https://reviews.apache.org/r/34040/#comment134165>
d can't be null here.
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java
<https://reviews.apache.org/r/34040/#comment134166>
d can't be null here.
- Ashutosh Chauhan
On May 11, 2015, 2:24 p.m., Aihua Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34040/
> -----------------------------------------------------------
>
> (Updated May 11, 2015, 2:24 p.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This is the first step to add sum() support for "rows between x preceding and y preceding" and "rows between x following and y following" windowing.
>
> This is just a refactoring without affecting the functionality by passing WindowFrameDef instead of passing # of preceding and # of following.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/PTFTranslator.java 00b43c6
> ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/BoundaryDef.java f692fa2
> ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/WindowFrameDef.java e08bdd5
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java 12a327f
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFFirstValue.java f679387
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFLastValue.java e099154
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFMax.java a153818
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFStreamingEvaluator.java d68c085
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java ffb7093
> ql/src/test/org/apache/hadoop/hive/ql/udaf/TestStreamingSum.java a331e66
>
> Diff: https://reviews.apache.org/r/34040/diff/
>
>
> Testing
> -------
>
> Testing has been done and the unit tests failures seem to be unrelated.
>
> https://issues.apache.org/jira/browse/HIVE-10643
>
>
> Thanks,
>
> Aihua Xu
>
>