You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Remus Rusanu <re...@microsoft.com> on 2014/02/05 12:04:39 UTC

Review Request 17737: Add DECIMAL support to vectorized group by operator

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

Review request for hive, Eric Hanson and Jitendra Pandey.


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


Repository: hive-git


Description
-------

Implements Decimal aggregate operators, decimal vector hash key wrapper, extends vectorizer to support decimal in GBY.


Diffs
-----

  ant/src/org/apache/hadoop/hive/ant/GenVectorCode.java 1b76fc9 
  common/src/java/org/apache/hadoop/hive/common/type/Decimal128.java 2e0f058 
  common/src/java/org/apache/hadoop/hive/common/type/HiveDecimal.java 29c5168 
  common/src/java/org/apache/hadoop/hive/common/type/UnsignedInt128.java fb3c346 
  common/src/java/org/apache/hive/common/util/Decimal128FastBuffer.java PRE-CREATION 
  ql/src/gen/vectorization/UDAFTemplates/VectorUDAFMinMaxDecimal.txt PRE-CREATION 
  ql/src/gen/vectorization/UDAFTemplates/VectorUDAFVarDecimal.txt PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorHashKeyWrapper.java f083d86 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorHashKeyWrapperBatch.java e978110 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java f5ab731 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizedRowBatchCtx.java f513188 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpressionWriter.java e5c3aa4 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpressionWriterFactory.java a242fef 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/aggregates/VectorUDAFAvgDecimal.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/aggregates/VectorUDAFSumDecimal.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java ad96fa5 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java 1a00800 
  ql/src/test/org/apache/hadoop/hive/ql/exec/vector/TestVectorGroupByOperator.java a2b45f8 
  ql/src/test/org/apache/hadoop/hive/ql/exec/vector/util/FakeVectorRowBatchFromObjectIterables.java c8eaea1 
  serde/src/test/org/apache/hadoop/hive/serde2/io/TestHiveDecimalWritable.java PRE-CREATION 

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


Testing
-------

New unit tests added, manually tested decimal GBY queries


Thanks,

Remus Rusanu


Re: Review Request 17737: Add DECIMAL support to vectorized group by operator

Posted by Remus Rusanu <re...@microsoft.com>.

> On Feb. 10, 2014, 9:58 p.m., Jitendra Pandey wrote:
> > ql/src/gen/vectorization/UDAFTemplates/VectorUDAFMinMaxDecimal.txt, line 53
> > <https://reviews.apache.org/r/17737/diff/1/?file=470006#file470006line53>
> >
> >     Should we initialize isNull to true? It seems it will always be false otherwise.

It is initialized to true explicitly @407 in  public void reset(AggregationBuffer agg). This pattern in repeated in all aggregates, the aggreagt structures are always explictly initialized before use. I agree it should be at least documented in a comment, and did so.


- Remus


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


On Feb. 5, 2014, 11:04 a.m., Remus Rusanu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17737/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2014, 11:04 a.m.)
> 
> 
> Review request for hive, Eric Hanson and Jitendra Pandey.
> 
> 
> Bugs: HIVE-6344
>     https://issues.apache.org/jira/browse/HIVE-6344
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Implements Decimal aggregate operators, decimal vector hash key wrapper, extends vectorizer to support decimal in GBY.
> 
> 
> Diffs
> -----
> 
>   ant/src/org/apache/hadoop/hive/ant/GenVectorCode.java 1b76fc9 
>   common/src/java/org/apache/hadoop/hive/common/type/Decimal128.java 2e0f058 
>   common/src/java/org/apache/hadoop/hive/common/type/HiveDecimal.java 29c5168 
>   common/src/java/org/apache/hadoop/hive/common/type/UnsignedInt128.java fb3c346 
>   common/src/java/org/apache/hive/common/util/Decimal128FastBuffer.java PRE-CREATION 
>   ql/src/gen/vectorization/UDAFTemplates/VectorUDAFMinMaxDecimal.txt PRE-CREATION 
>   ql/src/gen/vectorization/UDAFTemplates/VectorUDAFVarDecimal.txt PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorHashKeyWrapper.java f083d86 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorHashKeyWrapperBatch.java e978110 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java f5ab731 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizedRowBatchCtx.java f513188 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpressionWriter.java e5c3aa4 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpressionWriterFactory.java a242fef 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/aggregates/VectorUDAFAvgDecimal.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/aggregates/VectorUDAFSumDecimal.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java ad96fa5 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java 1a00800 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/vector/TestVectorGroupByOperator.java a2b45f8 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/vector/util/FakeVectorRowBatchFromObjectIterables.java c8eaea1 
>   serde/src/test/org/apache/hadoop/hive/serde2/io/TestHiveDecimalWritable.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/17737/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added, manually tested decimal GBY queries
> 
> 
> Thanks,
> 
> Remus Rusanu
> 
>


Re: Review Request 17737: Add DECIMAL support to vectorized group by operator

Posted by Jitendra Pandey <ji...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17737/#review33964
-----------------------------------------------------------



common/src/java/org/apache/hadoop/hive/common/type/UnsignedInt128.java
<https://reviews.apache.org/r/17737/#comment63819>

    Tab characters and indentation need fix.



common/src/java/org/apache/hadoop/hive/common/type/UnsignedInt128.java
<https://reviews.apache.org/r/17737/#comment63820>

    Indentation needs to be fixed.



ql/src/gen/vectorization/UDAFTemplates/VectorUDAFMinMaxDecimal.txt
<https://reviews.apache.org/r/17737/#comment63849>

    Should we initialize isNull to true? It seems it will always be false otherwise.



ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorHashKeyWrapperBatch.java
<https://reviews.apache.org/r/17737/#comment64067>

    Please declare a constant for this number.



ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java
<https://reviews.apache.org/r/17737/#comment64068>

    HIVE-6333 modifies this method to return decimal type along with precision and scale e.g. decimal(5,3).


- Jitendra Pandey


On Feb. 5, 2014, 11:04 a.m., Remus Rusanu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17737/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2014, 11:04 a.m.)
> 
> 
> Review request for hive, Eric Hanson and Jitendra Pandey.
> 
> 
> Bugs: HIVE-6344
>     https://issues.apache.org/jira/browse/HIVE-6344
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Implements Decimal aggregate operators, decimal vector hash key wrapper, extends vectorizer to support decimal in GBY.
> 
> 
> Diffs
> -----
> 
>   ant/src/org/apache/hadoop/hive/ant/GenVectorCode.java 1b76fc9 
>   common/src/java/org/apache/hadoop/hive/common/type/Decimal128.java 2e0f058 
>   common/src/java/org/apache/hadoop/hive/common/type/HiveDecimal.java 29c5168 
>   common/src/java/org/apache/hadoop/hive/common/type/UnsignedInt128.java fb3c346 
>   common/src/java/org/apache/hive/common/util/Decimal128FastBuffer.java PRE-CREATION 
>   ql/src/gen/vectorization/UDAFTemplates/VectorUDAFMinMaxDecimal.txt PRE-CREATION 
>   ql/src/gen/vectorization/UDAFTemplates/VectorUDAFVarDecimal.txt PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorHashKeyWrapper.java f083d86 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorHashKeyWrapperBatch.java e978110 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java f5ab731 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizedRowBatchCtx.java f513188 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpressionWriter.java e5c3aa4 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpressionWriterFactory.java a242fef 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/aggregates/VectorUDAFAvgDecimal.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/aggregates/VectorUDAFSumDecimal.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java ad96fa5 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java 1a00800 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/vector/TestVectorGroupByOperator.java a2b45f8 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/vector/util/FakeVectorRowBatchFromObjectIterables.java c8eaea1 
>   serde/src/test/org/apache/hadoop/hive/serde2/io/TestHiveDecimalWritable.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/17737/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added, manually tested decimal GBY queries
> 
> 
> Thanks,
> 
> Remus Rusanu
> 
>


Re: Review Request 17737: Add DECIMAL support to vectorized group by operator

Posted by Remus Rusanu <re...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17737/
-----------------------------------------------------------

(Updated Feb. 5, 2014, 11:04 a.m.)


Review request for hive, Eric Hanson and Jitendra Pandey.


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


Repository: hive-git


Description
-------

Implements Decimal aggregate operators, decimal vector hash key wrapper, extends vectorizer to support decimal in GBY.


Diffs
-----

  ant/src/org/apache/hadoop/hive/ant/GenVectorCode.java 1b76fc9 
  common/src/java/org/apache/hadoop/hive/common/type/Decimal128.java 2e0f058 
  common/src/java/org/apache/hadoop/hive/common/type/HiveDecimal.java 29c5168 
  common/src/java/org/apache/hadoop/hive/common/type/UnsignedInt128.java fb3c346 
  common/src/java/org/apache/hive/common/util/Decimal128FastBuffer.java PRE-CREATION 
  ql/src/gen/vectorization/UDAFTemplates/VectorUDAFMinMaxDecimal.txt PRE-CREATION 
  ql/src/gen/vectorization/UDAFTemplates/VectorUDAFVarDecimal.txt PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorHashKeyWrapper.java f083d86 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorHashKeyWrapperBatch.java e978110 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java f5ab731 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizedRowBatchCtx.java f513188 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpressionWriter.java e5c3aa4 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpressionWriterFactory.java a242fef 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/aggregates/VectorUDAFAvgDecimal.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/aggregates/VectorUDAFSumDecimal.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java ad96fa5 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java 1a00800 
  ql/src/test/org/apache/hadoop/hive/ql/exec/vector/TestVectorGroupByOperator.java a2b45f8 
  ql/src/test/org/apache/hadoop/hive/ql/exec/vector/util/FakeVectorRowBatchFromObjectIterables.java c8eaea1 
  serde/src/test/org/apache/hadoop/hive/serde2/io/TestHiveDecimalWritable.java PRE-CREATION 

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


Testing
-------

New unit tests added, manually tested decimal GBY queries


Thanks,

Remus Rusanu