You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Xuefu Zhang <xz...@cloudera.com> on 2014/02/25 20:37:13 UTC

Review Request 18478: HIVE-6459: Change the precison/scale for intermediate sum result in the avg() udf

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

Review request for hive.


Bugs: HIVE-6459
    https://issues.apache.org/jira/browse/HIVE-6459


Repository: hive-git


Description
-------

Patch addressed the issue by keeping the type of the sum field consistent with that of sum UDF. The type of the final avg result is unchanged.


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/aggregates/VectorUDAFAvgDecimal.java 6f593f9 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java abd54be 
  ql/src/test/queries/clientpositive/vector_decimal_aggregate.q eb9146e 
  ql/src/test/results/clientpositive/create_genericudaf.q.out 96fe2fa 
  ql/src/test/results/clientpositive/decimal_precision.q.out a80695c 
  ql/src/test/results/clientpositive/decimal_udf.q.out 74ae554 
  ql/src/test/results/clientpositive/groupby10.q.out 341427f 
  ql/src/test/results/clientpositive/groupby3.q.out a74f2b5 
  ql/src/test/results/clientpositive/groupby3_map.q.out 9424071 
  ql/src/test/results/clientpositive/groupby3_map_multi_distinct.q.out 9bcd7c9 
  ql/src/test/results/clientpositive/groupby3_map_skew.q.out f438f89 
  ql/src/test/results/clientpositive/groupby_grouping_sets3.q.out 310a202 
  ql/src/test/results/clientpositive/limit_pushdown.q.out a8add4c 
  ql/src/test/results/clientpositive/subquery_in.q.out 48be22b 
  ql/src/test/results/clientpositive/subquery_in_having.q.out ef3dc18 
  ql/src/test/results/clientpositive/subquery_notin.q.out b2d687b 
  ql/src/test/results/clientpositive/subquery_notin_having.q.out 5f4d96e 
  ql/src/test/results/clientpositive/udaf_number_format.q.out 339ef94 
  ql/src/test/results/clientpositive/udf3.q.out 546f949 
  ql/src/test/results/clientpositive/udf8.q.out 79c3bff 
  ql/src/test/results/clientpositive/vector_decimal_aggregate.q.out 8b73971 
  ql/src/test/results/clientpositive/vectorization_limit.q.out 51a4e81 
  ql/src/test/results/clientpositive/vectorization_pushdown.q.out df474d6 
  ql/src/test/results/clientpositive/vectorization_short_regress.q.out 07accb6 
  ql/src/test/results/clientpositive/vectorized_mapjoin.q.out 9590642 
  ql/src/test/results/clientpositive/vectorized_shufflejoin.q.out 928bc82 
  ql/src/test/results/compiler/plan/groupby3.q.xml cc88d5c 

Diff: https://reviews.apache.org/r/18478/diff/


Testing
-------

Existing tests cover this. Some test output is regenerated due to the output diff.


Thanks,

Xuefu Zhang


Re: Review Request 18478: HIVE-6459: Change the precison/scale for intermediate sum result in the avg() udf

Posted by Xuefu Zhang <xz...@cloudera.com>.

> On Feb. 28, 2014, 7:51 p.m., Prasad Mujumdar wrote:
> > ql/src/test/queries/clientpositive/vector_decimal_aggregate.q, line 7
> > <https://reviews.apache.org/r/18478/diff/1/?file=503529#file503529line7>
> >
> >     Is the vectorization disabled intentionally ?

I guess it's by accident. Thank for catching this. I will fix this.


- Xuefu


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


On Feb. 25, 2014, 7:37 p.m., Xuefu Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18478/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2014, 7:37 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6459
>     https://issues.apache.org/jira/browse/HIVE-6459
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Patch addressed the issue by keeping the type of the sum field consistent with that of sum UDF. The type of the final avg result is unchanged.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/aggregates/VectorUDAFAvgDecimal.java 6f593f9 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java abd54be 
>   ql/src/test/queries/clientpositive/vector_decimal_aggregate.q eb9146e 
>   ql/src/test/results/clientpositive/create_genericudaf.q.out 96fe2fa 
>   ql/src/test/results/clientpositive/decimal_precision.q.out a80695c 
>   ql/src/test/results/clientpositive/decimal_udf.q.out 74ae554 
>   ql/src/test/results/clientpositive/groupby10.q.out 341427f 
>   ql/src/test/results/clientpositive/groupby3.q.out a74f2b5 
>   ql/src/test/results/clientpositive/groupby3_map.q.out 9424071 
>   ql/src/test/results/clientpositive/groupby3_map_multi_distinct.q.out 9bcd7c9 
>   ql/src/test/results/clientpositive/groupby3_map_skew.q.out f438f89 
>   ql/src/test/results/clientpositive/groupby_grouping_sets3.q.out 310a202 
>   ql/src/test/results/clientpositive/limit_pushdown.q.out a8add4c 
>   ql/src/test/results/clientpositive/subquery_in.q.out 48be22b 
>   ql/src/test/results/clientpositive/subquery_in_having.q.out ef3dc18 
>   ql/src/test/results/clientpositive/subquery_notin.q.out b2d687b 
>   ql/src/test/results/clientpositive/subquery_notin_having.q.out 5f4d96e 
>   ql/src/test/results/clientpositive/udaf_number_format.q.out 339ef94 
>   ql/src/test/results/clientpositive/udf3.q.out 546f949 
>   ql/src/test/results/clientpositive/udf8.q.out 79c3bff 
>   ql/src/test/results/clientpositive/vector_decimal_aggregate.q.out 8b73971 
>   ql/src/test/results/clientpositive/vectorization_limit.q.out 51a4e81 
>   ql/src/test/results/clientpositive/vectorization_pushdown.q.out df474d6 
>   ql/src/test/results/clientpositive/vectorization_short_regress.q.out 07accb6 
>   ql/src/test/results/clientpositive/vectorized_mapjoin.q.out 9590642 
>   ql/src/test/results/clientpositive/vectorized_shufflejoin.q.out 928bc82 
>   ql/src/test/results/compiler/plan/groupby3.q.xml cc88d5c 
> 
> Diff: https://reviews.apache.org/r/18478/diff/
> 
> 
> Testing
> -------
> 
> Existing tests cover this. Some test output is regenerated due to the output diff.
> 
> 
> Thanks,
> 
> Xuefu Zhang
> 
>


Re: Review Request 18478: HIVE-6459: Change the precison/scale for intermediate sum result in the avg() udf

Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18478/#review35825
-----------------------------------------------------------

Ship it!


Looks fine to me.
A minor question below.


ql/src/test/queries/clientpositive/vector_decimal_aggregate.q
<https://reviews.apache.org/r/18478/#comment66575>

    Is the vectorization disabled intentionally ?


- Prasad Mujumdar


On Feb. 25, 2014, 7:37 p.m., Xuefu Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18478/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2014, 7:37 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6459
>     https://issues.apache.org/jira/browse/HIVE-6459
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Patch addressed the issue by keeping the type of the sum field consistent with that of sum UDF. The type of the final avg result is unchanged.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/aggregates/VectorUDAFAvgDecimal.java 6f593f9 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java abd54be 
>   ql/src/test/queries/clientpositive/vector_decimal_aggregate.q eb9146e 
>   ql/src/test/results/clientpositive/create_genericudaf.q.out 96fe2fa 
>   ql/src/test/results/clientpositive/decimal_precision.q.out a80695c 
>   ql/src/test/results/clientpositive/decimal_udf.q.out 74ae554 
>   ql/src/test/results/clientpositive/groupby10.q.out 341427f 
>   ql/src/test/results/clientpositive/groupby3.q.out a74f2b5 
>   ql/src/test/results/clientpositive/groupby3_map.q.out 9424071 
>   ql/src/test/results/clientpositive/groupby3_map_multi_distinct.q.out 9bcd7c9 
>   ql/src/test/results/clientpositive/groupby3_map_skew.q.out f438f89 
>   ql/src/test/results/clientpositive/groupby_grouping_sets3.q.out 310a202 
>   ql/src/test/results/clientpositive/limit_pushdown.q.out a8add4c 
>   ql/src/test/results/clientpositive/subquery_in.q.out 48be22b 
>   ql/src/test/results/clientpositive/subquery_in_having.q.out ef3dc18 
>   ql/src/test/results/clientpositive/subquery_notin.q.out b2d687b 
>   ql/src/test/results/clientpositive/subquery_notin_having.q.out 5f4d96e 
>   ql/src/test/results/clientpositive/udaf_number_format.q.out 339ef94 
>   ql/src/test/results/clientpositive/udf3.q.out 546f949 
>   ql/src/test/results/clientpositive/udf8.q.out 79c3bff 
>   ql/src/test/results/clientpositive/vector_decimal_aggregate.q.out 8b73971 
>   ql/src/test/results/clientpositive/vectorization_limit.q.out 51a4e81 
>   ql/src/test/results/clientpositive/vectorization_pushdown.q.out df474d6 
>   ql/src/test/results/clientpositive/vectorization_short_regress.q.out 07accb6 
>   ql/src/test/results/clientpositive/vectorized_mapjoin.q.out 9590642 
>   ql/src/test/results/clientpositive/vectorized_shufflejoin.q.out 928bc82 
>   ql/src/test/results/compiler/plan/groupby3.q.xml cc88d5c 
> 
> Diff: https://reviews.apache.org/r/18478/diff/
> 
> 
> Testing
> -------
> 
> Existing tests cover this. Some test output is regenerated due to the output diff.
> 
> 
> Thanks,
> 
> Xuefu Zhang
> 
>