hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Aihua Xu" <...@cloudera.com>
Subject 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)
Date Mon, 11 May 2015 20:08:32 GMT


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


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