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