Re: Review Request 65500: HIVE-18421 : Vectorized execution handles overflows in a different manner than non-vectorized execution

2018-02-12 Thread Vihang Karajgaonkar via Review Board 


> On Feb. 8, 2018, 3:51 a.m., Aihua Xu wrote:
> > ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticColumn.txt
> > Lines 131 (patched)
> > 
> >
> > Can we remove #ELSE from here and some other places below? Seems not 
> > needed.

The #ELSE is needed since it is used for the generating the unchecked variants 
of the vector expressions.


- Vihang


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


On Feb. 7, 2018, 7:13 a.m., Vihang Karajgaonkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65500/
> ---
> 
> (Updated Feb. 7, 2018, 7:13 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Gopal V, Matt McCline, and Sahil Takiar.
> 
> 
> Bugs: HIVE-18421
> https://issues.apache.org/jira/browse/HIVE-18421
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See JIRA.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 99e8457c7b2d506cfc7c71ca18bc678fe5cdf049 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticColumn.txt 
> b5011c3adcedf8974d3241994733e0021a851cbd 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticScalar.txt 
> cbec1abcc2b66f3ffc91b4778daf5017eff4379d 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnDivideColumn.txt 
> 3e955578933dd7990939865527c3bd11023b3a90 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnUnaryMinus.txt 
> f0ab4711e79c8a1bfceebcde9a3dda2b4e15a38a 
>   ql/src/gen/vectorization/ExpressionTemplates/ScalarArithmeticColumn.txt 
> e95baa6199e138a4e0c009e62ce495b626e5909c 
>   ql/src/gen/vectorization/TestTemplates/TestClass.txt 
> 62c58fb293fbe2d4d948c6a3409ee31466424a02 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnColumnOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnScalarOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorExpressionDescriptor.java
>  bbe78c8720e16163b642f54d27fdf6b65ba9850b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
> 8264e8ad285deac29424bd1cb0bf626436d47c75 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/LongColModuloLongColumnChecked.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/OverflowUtils.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModDoubleToDouble.java
>  75ec419aa9ea5c3fcc5e7314fbac756d6a5d36d5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModLongToLong.java
>  6b4d714c9a79a55593c4a4d254267a3035abb10f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpression.java
>  710165033627b33d9b238cc847dbac36c07ee5f6 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMinus.java 
> af8552caa02f2896f393a5099abdb1ae5abd4c16 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMod.java 
> e2a638da518a2071ff15b8da6899646ec45c832a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMultiply.java 
> 99d1ad7f203d946fd89d26074bd0e00dec8b3a1a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java 
> 4e45788936559bbb7cfe65e9ffd083747b37dcc2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPPlus.java 
> b1200e673e6b470b5fd1cc856270a6da615f16cb 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestUnaryMinus.java
>  ab6f6b79316818cac458390dc2d087091057c63b 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorArithmeticExpressions.java
>  02dec659ce421eef06f924bb6973070878d57be3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorMathFunctions.java
>  1950e92bd6d5e4f818588e691db30cd28193c716 
>   ql/src/test/queries/clientpositive/vectorization_numeric_overflows.q 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/vectorization_numeric_overflows.q.out 
> PRE-CREATION 
>   vector-code-gen/src/org/apache/hadoop/hive/tools/GenVectorCode.java 
> 657ea34e11f7465e6c77d45128b298e7326a057b 
>   vector-code-gen/src/org/apache/hadoop/hive/tools/GenVectorTestCode.java 
> d97646f8b1c4a074da59b4685939fc4359c9c30d 
> 
> 
> Diff: https://reviews.apache.org/r/65500/diff/6/
> 
> 
> Testing
> ---
> 
> HiveQA
> 
> 
> Thanks,
> 
> Vihang Karajgaonkar
> 
>

Re: Review Request 65500: HIVE-18421 : Vectorized execution handles overflows in a different manner than non-vectorized execution

2018-02-07 Thread Aihua Xu via Review Board 

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




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 2980 (patched)


I'm confused that we need this configuration. So we need to manually verify 
if the data is overflow and if there is overflow, we turn that on? 

Even we want to provide both CHECKED and UNCHECKED, I think we need to fail 
the query if there is overflow for unchecked implementation, otherwise, the 
user may get incorrect result. 

Of course, to me, I think we should just have CHECKED implementation, that 
is, we should always handle overflow internally.

BTW: how much performance impact will it be? And why do we  have big 
performance impact from the CHECKED implementation (sorry, I don't follow 
exactly the vecorization work)?



ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticColumn.txt
Lines 131 (patched)


Can we remove #ELSE from here and some other places below? Seems not needed.


- Aihua Xu


On Feb. 7, 2018, 7:13 a.m., Vihang Karajgaonkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65500/
> ---
> 
> (Updated Feb. 7, 2018, 7:13 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Gopal V, Matt McCline, and Sahil Takiar.
> 
> 
> Bugs: HIVE-18421
> https://issues.apache.org/jira/browse/HIVE-18421
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See JIRA.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 99e8457c7b2d506cfc7c71ca18bc678fe5cdf049 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticColumn.txt 
> b5011c3adcedf8974d3241994733e0021a851cbd 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticScalar.txt 
> cbec1abcc2b66f3ffc91b4778daf5017eff4379d 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnDivideColumn.txt 
> 3e955578933dd7990939865527c3bd11023b3a90 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnUnaryMinus.txt 
> f0ab4711e79c8a1bfceebcde9a3dda2b4e15a38a 
>   ql/src/gen/vectorization/ExpressionTemplates/ScalarArithmeticColumn.txt 
> e95baa6199e138a4e0c009e62ce495b626e5909c 
>   ql/src/gen/vectorization/TestTemplates/TestClass.txt 
> 62c58fb293fbe2d4d948c6a3409ee31466424a02 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnColumnOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnScalarOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorExpressionDescriptor.java
>  bbe78c8720e16163b642f54d27fdf6b65ba9850b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
> 8264e8ad285deac29424bd1cb0bf626436d47c75 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/LongColModuloLongColumnChecked.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/OverflowUtils.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModDoubleToDouble.java
>  75ec419aa9ea5c3fcc5e7314fbac756d6a5d36d5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModLongToLong.java
>  6b4d714c9a79a55593c4a4d254267a3035abb10f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpression.java
>  710165033627b33d9b238cc847dbac36c07ee5f6 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMinus.java 
> af8552caa02f2896f393a5099abdb1ae5abd4c16 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMod.java 
> e2a638da518a2071ff15b8da6899646ec45c832a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMultiply.java 
> 99d1ad7f203d946fd89d26074bd0e00dec8b3a1a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java 
> 4e45788936559bbb7cfe65e9ffd083747b37dcc2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPPlus.java 
> b1200e673e6b470b5fd1cc856270a6da615f16cb 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestUnaryMinus.java
>  ab6f6b79316818cac458390dc2d087091057c63b 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorArithmeticExpressions.java
>  02dec659ce421eef06f924bb6973070878d57be3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorMathFunctions.java
>  1950e92bd6d5e4f818588e691db30cd28193c716 
>   ql/src/test/queries/clientpositive/vectorization_numeric_overflows.q 
> PRE-CREATION 
>

Re: Review Request 65500: HIVE-18421 : Vectorized execution handles overflows in a different manner than non-vectorized execution

2018-02-06 Thread Vihang Karajgaonkar via Review Board 


> On Feb. 7, 2018, 2:40 a.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModLongToLong.java
> > Lines 46-48 (patched)
> > 
> >
> > is there a way to do this check just once rather than for each value?

Made it slightly better by removing the need for null check and adding method 
call in getTypeName(). You would still need a switch on the typename though. 
Having a couple of booleans can get rid of switch but in terms of number of 
comparisons it would still be at the same cost (in fact switch is better in 
this case I believe)


- Vihang


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


On Feb. 7, 2018, 7:13 a.m., Vihang Karajgaonkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65500/
> ---
> 
> (Updated Feb. 7, 2018, 7:13 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Gopal V, Matt McCline, and Sahil Takiar.
> 
> 
> Bugs: HIVE-18421
> https://issues.apache.org/jira/browse/HIVE-18421
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See JIRA.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 99e8457c7b2d506cfc7c71ca18bc678fe5cdf049 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticColumn.txt 
> b5011c3adcedf8974d3241994733e0021a851cbd 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticScalar.txt 
> cbec1abcc2b66f3ffc91b4778daf5017eff4379d 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnDivideColumn.txt 
> 3e955578933dd7990939865527c3bd11023b3a90 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnUnaryMinus.txt 
> f0ab4711e79c8a1bfceebcde9a3dda2b4e15a38a 
>   ql/src/gen/vectorization/ExpressionTemplates/ScalarArithmeticColumn.txt 
> e95baa6199e138a4e0c009e62ce495b626e5909c 
>   ql/src/gen/vectorization/TestTemplates/TestClass.txt 
> 62c58fb293fbe2d4d948c6a3409ee31466424a02 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnColumnOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnScalarOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorExpressionDescriptor.java
>  bbe78c8720e16163b642f54d27fdf6b65ba9850b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
> 8264e8ad285deac29424bd1cb0bf626436d47c75 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/LongColModuloLongColumnChecked.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/OverflowUtils.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModDoubleToDouble.java
>  75ec419aa9ea5c3fcc5e7314fbac756d6a5d36d5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModLongToLong.java
>  6b4d714c9a79a55593c4a4d254267a3035abb10f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpression.java
>  710165033627b33d9b238cc847dbac36c07ee5f6 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMinus.java 
> af8552caa02f2896f393a5099abdb1ae5abd4c16 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMod.java 
> e2a638da518a2071ff15b8da6899646ec45c832a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMultiply.java 
> 99d1ad7f203d946fd89d26074bd0e00dec8b3a1a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java 
> 4e45788936559bbb7cfe65e9ffd083747b37dcc2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPPlus.java 
> b1200e673e6b470b5fd1cc856270a6da615f16cb 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestUnaryMinus.java
>  ab6f6b79316818cac458390dc2d087091057c63b 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorArithmeticExpressions.java
>  02dec659ce421eef06f924bb6973070878d57be3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorMathFunctions.java
>  1950e92bd6d5e4f818588e691db30cd28193c716 
>   ql/src/test/queries/clientpositive/vectorization_numeric_overflows.q 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/vectorization_numeric_overflows.q.out 
> PRE-CREATION 
>   vector-code-gen/src/org/apache/hadoop/hive/tools/GenVectorCode.java 
> 657ea34e11f7465e6c77d45128b298e7326a057b 
>   vector-code-gen/src/org/apache/hadoop/hive/tools/GenVectorTestCode.java 
> d97646f8b1c4a074da59b4685939fc4359c9c30d 
> 
> 
> Diff: https://reviews.apache.org/r/65500/diff/6/
> 
> 
> Testing
> ---
> 
>

Re: Review Request 65500: HIVE-18421 : Vectorized execution handles overflows in a different manner than non-vectorized execution

2018-02-06 Thread Vihang Karajgaonkar via Review Board 

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

(Updated Feb. 7, 2018, 7:13 a.m.)


Review request for hive, Aihua Xu, Gopal V, Matt McCline, and Sahil Takiar.


Changes
---

added Sahil's suggested changes.


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


Repository: hive-git


Description
---

See JIRA.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
99e8457c7b2d506cfc7c71ca18bc678fe5cdf049 
  ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticColumn.txt 
b5011c3adcedf8974d3241994733e0021a851cbd 
  ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticScalar.txt 
cbec1abcc2b66f3ffc91b4778daf5017eff4379d 
  ql/src/gen/vectorization/ExpressionTemplates/ColumnDivideColumn.txt 
3e955578933dd7990939865527c3bd11023b3a90 
  ql/src/gen/vectorization/ExpressionTemplates/ColumnUnaryMinus.txt 
f0ab4711e79c8a1bfceebcde9a3dda2b4e15a38a 
  ql/src/gen/vectorization/ExpressionTemplates/ScalarArithmeticColumn.txt 
e95baa6199e138a4e0c009e62ce495b626e5909c 
  ql/src/gen/vectorization/TestTemplates/TestClass.txt 
62c58fb293fbe2d4d948c6a3409ee31466424a02 
  
ql/src/gen/vectorization/TestTemplates/TestColumnColumnOperationVectorExpressionCheckedEvaluation.txt
 PRE-CREATION 
  
ql/src/gen/vectorization/TestTemplates/TestColumnScalarOperationVectorExpressionCheckedEvaluation.txt
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorExpressionDescriptor.java
 bbe78c8720e16163b642f54d27fdf6b65ba9850b 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
8264e8ad285deac29424bd1cb0bf626436d47c75 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/LongColModuloLongColumnChecked.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/OverflowUtils.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModDoubleToDouble.java
 75ec419aa9ea5c3fcc5e7314fbac756d6a5d36d5 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModLongToLong.java
 6b4d714c9a79a55593c4a4d254267a3035abb10f 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpression.java
 710165033627b33d9b238cc847dbac36c07ee5f6 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMinus.java 
af8552caa02f2896f393a5099abdb1ae5abd4c16 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMod.java 
e2a638da518a2071ff15b8da6899646ec45c832a 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMultiply.java 
99d1ad7f203d946fd89d26074bd0e00dec8b3a1a 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java 
4e45788936559bbb7cfe65e9ffd083747b37dcc2 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPPlus.java 
b1200e673e6b470b5fd1cc856270a6da615f16cb 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestUnaryMinus.java
 ab6f6b79316818cac458390dc2d087091057c63b 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorArithmeticExpressions.java
 02dec659ce421eef06f924bb6973070878d57be3 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorMathFunctions.java
 1950e92bd6d5e4f818588e691db30cd28193c716 
  ql/src/test/queries/clientpositive/vectorization_numeric_overflows.q 
PRE-CREATION 
  ql/src/test/results/clientpositive/vectorization_numeric_overflows.q.out 
PRE-CREATION 
  vector-code-gen/src/org/apache/hadoop/hive/tools/GenVectorCode.java 
657ea34e11f7465e6c77d45128b298e7326a057b 
  vector-code-gen/src/org/apache/hadoop/hive/tools/GenVectorTestCode.java 
d97646f8b1c4a074da59b4685939fc4359c9c30d 


Diff: https://reviews.apache.org/r/65500/diff/6/

Changes: https://reviews.apache.org/r/65500/diff/5-6/


Testing
---

HiveQA


Thanks,

Vihang Karajgaonkar

Re: Review Request 65500: HIVE-18421 : Vectorized execution handles overflows in a different manner than non-vectorized execution

2018-02-06 Thread Sahil Takiar 

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


Fix it, then Ship it!




One last comment, otherwise LGTM.


ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModLongToLong.java
Lines 46-48 (patched)


is there a way to do this check just once rather than for each value?


- Sahil Takiar


On Feb. 7, 2018, 12:49 a.m., Vihang Karajgaonkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65500/
> ---
> 
> (Updated Feb. 7, 2018, 12:49 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Gopal V, Matt McCline, and Sahil Takiar.
> 
> 
> Bugs: HIVE-18421
> https://issues.apache.org/jira/browse/HIVE-18421
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See JIRA.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 99e8457c7b2d506cfc7c71ca18bc678fe5cdf049 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticColumn.txt 
> b5011c3adcedf8974d3241994733e0021a851cbd 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticScalar.txt 
> cbec1abcc2b66f3ffc91b4778daf5017eff4379d 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnDivideColumn.txt 
> 3e955578933dd7990939865527c3bd11023b3a90 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnUnaryMinus.txt 
> f0ab4711e79c8a1bfceebcde9a3dda2b4e15a38a 
>   ql/src/gen/vectorization/ExpressionTemplates/ScalarArithmeticColumn.txt 
> e95baa6199e138a4e0c009e62ce495b626e5909c 
>   ql/src/gen/vectorization/TestTemplates/TestClass.txt 
> 62c58fb293fbe2d4d948c6a3409ee31466424a02 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnColumnOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnScalarOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorExpressionDescriptor.java
>  bbe78c8720e16163b642f54d27fdf6b65ba9850b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
> 8264e8ad285deac29424bd1cb0bf626436d47c75 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/LongColModuloLongColumnChecked.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/OverflowUtils.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModDoubleToDouble.java
>  75ec419aa9ea5c3fcc5e7314fbac756d6a5d36d5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModLongToLong.java
>  6b4d714c9a79a55593c4a4d254267a3035abb10f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpression.java
>  710165033627b33d9b238cc847dbac36c07ee5f6 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMinus.java 
> af8552caa02f2896f393a5099abdb1ae5abd4c16 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMod.java 
> e2a638da518a2071ff15b8da6899646ec45c832a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMultiply.java 
> 99d1ad7f203d946fd89d26074bd0e00dec8b3a1a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java 
> 4e45788936559bbb7cfe65e9ffd083747b37dcc2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPPlus.java 
> b1200e673e6b470b5fd1cc856270a6da615f16cb 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestUnaryMinus.java
>  ab6f6b79316818cac458390dc2d087091057c63b 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorArithmeticExpressions.java
>  02dec659ce421eef06f924bb6973070878d57be3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorMathFunctions.java
>  1950e92bd6d5e4f818588e691db30cd28193c716 
>   ql/src/test/queries/clientpositive/vectorization_numeric_overflows.q 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/vectorization_numeric_overflows.q.out 
> PRE-CREATION 
>   vector-code-gen/src/org/apache/hadoop/hive/tools/GenVectorCode.java 
> 657ea34e11f7465e6c77d45128b298e7326a057b 
>   vector-code-gen/src/org/apache/hadoop/hive/tools/GenVectorTestCode.java 
> d97646f8b1c4a074da59b4685939fc4359c9c30d 
> 
> 
> Diff: https://reviews.apache.org/r/65500/diff/5/
> 
> 
> Testing
> ---
> 
> HiveQA
> 
> 
> Thanks,
> 
> Vihang Karajgaonkar
> 
>

Re: Review Request 65500: HIVE-18421 : Vectorized execution handles overflows in a different manner than non-vectorized execution

2018-02-06 Thread Vihang Karajgaonkar via Review Board 

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

(Updated Feb. 7, 2018, 12:49 a.m.)


Review request for hive, Aihua Xu, Gopal V, Matt McCline, and Sahil Takiar.


Changes
---

added changes suggested by Sahil.


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


Repository: hive-git


Description
---

See JIRA.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
99e8457c7b2d506cfc7c71ca18bc678fe5cdf049 
  ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticColumn.txt 
b5011c3adcedf8974d3241994733e0021a851cbd 
  ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticScalar.txt 
cbec1abcc2b66f3ffc91b4778daf5017eff4379d 
  ql/src/gen/vectorization/ExpressionTemplates/ColumnDivideColumn.txt 
3e955578933dd7990939865527c3bd11023b3a90 
  ql/src/gen/vectorization/ExpressionTemplates/ColumnUnaryMinus.txt 
f0ab4711e79c8a1bfceebcde9a3dda2b4e15a38a 
  ql/src/gen/vectorization/ExpressionTemplates/ScalarArithmeticColumn.txt 
e95baa6199e138a4e0c009e62ce495b626e5909c 
  ql/src/gen/vectorization/TestTemplates/TestClass.txt 
62c58fb293fbe2d4d948c6a3409ee31466424a02 
  
ql/src/gen/vectorization/TestTemplates/TestColumnColumnOperationVectorExpressionCheckedEvaluation.txt
 PRE-CREATION 
  
ql/src/gen/vectorization/TestTemplates/TestColumnScalarOperationVectorExpressionCheckedEvaluation.txt
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorExpressionDescriptor.java
 bbe78c8720e16163b642f54d27fdf6b65ba9850b 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
8264e8ad285deac29424bd1cb0bf626436d47c75 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/LongColModuloLongColumnChecked.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/OverflowUtils.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModDoubleToDouble.java
 75ec419aa9ea5c3fcc5e7314fbac756d6a5d36d5 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModLongToLong.java
 6b4d714c9a79a55593c4a4d254267a3035abb10f 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpression.java
 710165033627b33d9b238cc847dbac36c07ee5f6 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMinus.java 
af8552caa02f2896f393a5099abdb1ae5abd4c16 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMod.java 
e2a638da518a2071ff15b8da6899646ec45c832a 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMultiply.java 
99d1ad7f203d946fd89d26074bd0e00dec8b3a1a 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java 
4e45788936559bbb7cfe65e9ffd083747b37dcc2 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPPlus.java 
b1200e673e6b470b5fd1cc856270a6da615f16cb 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestUnaryMinus.java
 ab6f6b79316818cac458390dc2d087091057c63b 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorArithmeticExpressions.java
 02dec659ce421eef06f924bb6973070878d57be3 
  
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorMathFunctions.java
 1950e92bd6d5e4f818588e691db30cd28193c716 
  ql/src/test/queries/clientpositive/vectorization_numeric_overflows.q 
PRE-CREATION 
  ql/src/test/results/clientpositive/vectorization_numeric_overflows.q.out 
PRE-CREATION 
  vector-code-gen/src/org/apache/hadoop/hive/tools/GenVectorCode.java 
657ea34e11f7465e6c77d45128b298e7326a057b 
  vector-code-gen/src/org/apache/hadoop/hive/tools/GenVectorTestCode.java 
d97646f8b1c4a074da59b4685939fc4359c9c30d 


Diff: https://reviews.apache.org/r/65500/diff/4/

Changes: https://reviews.apache.org/r/65500/diff/3-4/


Testing
---

HiveQA


Thanks,

Vihang Karajgaonkar

Re: Review Request 65500: HIVE-18421 : Vectorized execution handles overflows in a different manner than non-vectorized execution

2018-02-06 Thread Vihang Karajgaonkar via Review Board 


> On Feb. 7, 2018, 12:17 a.m., Sahil Takiar wrote:
> > ql/src/gen/vectorization/TestTemplates/TestClass.txt
> > Lines 27 (patched)
> > 
> >
> > is this necessary?

yes, this is needed since this template is used to auto-generate the TestClass 
for vector expressions. The templates for test methods (eg 
TestColumnColumnOperationVectorExpressionCheckedEvaluation.txt) uses 
TypeInfoFactory, so we need the import


> On Feb. 7, 2018, 12:17 a.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/OverflowUtils.java
> > Lines 41 (patched)
> > 
> >
> > So would this be printed for each column vector batch? Wouldn't that 
> > flood the logs if there are a lot of rows being read?

fair point. I will remove this


> On Feb. 7, 2018, 12:17 a.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModDoubleToDouble.java
> > Lines 45-46 (patched)
> > 
> >
> > is the `func` method invoked for each row? does the content of the if 
> > statement change with each value of `v` passed into the method? or can the 
> > check be moved out and just evaluated once?
> > 
> > is it a valid case when the `outputTypeInfo` is `null`? shouldn't there 
> > always be an output type set?

based on my understanding outputTypeInfo should never be null. But I could be 
wrong and I thought its safer to do a null check. Also, many of the junit test 
cases don't set the outputTypeInfo. I will move if check out of this method so 
that it is evaluated only once.


- Vihang


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


On Feb. 7, 2018, 12:49 a.m., Vihang Karajgaonkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65500/
> ---
> 
> (Updated Feb. 7, 2018, 12:49 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Gopal V, Matt McCline, and Sahil Takiar.
> 
> 
> Bugs: HIVE-18421
> https://issues.apache.org/jira/browse/HIVE-18421
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See JIRA.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 99e8457c7b2d506cfc7c71ca18bc678fe5cdf049 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticColumn.txt 
> b5011c3adcedf8974d3241994733e0021a851cbd 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticScalar.txt 
> cbec1abcc2b66f3ffc91b4778daf5017eff4379d 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnDivideColumn.txt 
> 3e955578933dd7990939865527c3bd11023b3a90 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnUnaryMinus.txt 
> f0ab4711e79c8a1bfceebcde9a3dda2b4e15a38a 
>   ql/src/gen/vectorization/ExpressionTemplates/ScalarArithmeticColumn.txt 
> e95baa6199e138a4e0c009e62ce495b626e5909c 
>   ql/src/gen/vectorization/TestTemplates/TestClass.txt 
> 62c58fb293fbe2d4d948c6a3409ee31466424a02 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnColumnOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnScalarOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorExpressionDescriptor.java
>  bbe78c8720e16163b642f54d27fdf6b65ba9850b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
> 8264e8ad285deac29424bd1cb0bf626436d47c75 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/LongColModuloLongColumnChecked.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/OverflowUtils.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModDoubleToDouble.java
>  75ec419aa9ea5c3fcc5e7314fbac756d6a5d36d5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModLongToLong.java
>  6b4d714c9a79a55593c4a4d254267a3035abb10f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpression.java
>  710165033627b33d9b238cc847dbac36c07ee5f6 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMinus.java 
> af8552caa02f2896f393a5099abdb1ae5abd4c16 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMod.java 
> e2a638da518a2071ff15b8da6899646ec45c832a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMultiply.java 
> 99d1ad7f203d946fd89d26074bd0e00dec8b3a1a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java 
>

Re: Review Request 65500: HIVE-18421 : Vectorized execution handles overflows in a different manner than non-vectorized execution

2018-02-06 Thread Sahil Takiar 

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




ql/src/gen/vectorization/TestTemplates/TestClass.txt
Lines 27 (patched)


is this necessary?



ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/OverflowUtils.java
Lines 41 (patched)


So would this be printed for each column vector batch? Wouldn't that flood 
the logs if there are a lot of rows being read?



ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModDoubleToDouble.java
Lines 45-46 (patched)


is the `func` method invoked for each row? does the content of the if 
statement change with each value of `v` passed into the method? or can the 
check be moved out and just evaluated once?

is it a valid case when the `outputTypeInfo` is `null`? shouldn't there 
always be an output type set?


- Sahil Takiar


On Feb. 5, 2018, 8:54 p.m., Vihang Karajgaonkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65500/
> ---
> 
> (Updated Feb. 5, 2018, 8:54 p.m.)
> 
> 
> Review request for hive, Aihua Xu, Gopal V, Matt McCline, and Sahil Takiar.
> 
> 
> Bugs: HIVE-18421
> https://issues.apache.org/jira/browse/HIVE-18421
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See JIRA.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 99e8457c7b2d506cfc7c71ca18bc678fe5cdf049 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticColumn.txt 
> b5011c3adcedf8974d3241994733e0021a851cbd 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnArithmeticScalar.txt 
> cbec1abcc2b66f3ffc91b4778daf5017eff4379d 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnDivideColumn.txt 
> 3e955578933dd7990939865527c3bd11023b3a90 
>   ql/src/gen/vectorization/ExpressionTemplates/ColumnUnaryMinus.txt 
> f0ab4711e79c8a1bfceebcde9a3dda2b4e15a38a 
>   ql/src/gen/vectorization/ExpressionTemplates/ScalarArithmeticColumn.txt 
> e95baa6199e138a4e0c009e62ce495b626e5909c 
>   ql/src/gen/vectorization/TestTemplates/TestClass.txt 
> 62c58fb293fbe2d4d948c6a3409ee31466424a02 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnColumnOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/gen/vectorization/TestTemplates/TestColumnScalarOperationVectorExpressionCheckedEvaluation.txt
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorExpressionDescriptor.java
>  bbe78c8720e16163b642f54d27fdf6b65ba9850b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
> 8264e8ad285deac29424bd1cb0bf626436d47c75 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/LongColModuloLongColumnChecked.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/OverflowUtils.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModDoubleToDouble.java
>  75ec419aa9ea5c3fcc5e7314fbac756d6a5d36d5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/PosModLongToLong.java
>  6b4d714c9a79a55593c4a4d254267a3035abb10f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorExpression.java
>  710165033627b33d9b238cc847dbac36c07ee5f6 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMinus.java 
> af8552caa02f2896f393a5099abdb1ae5abd4c16 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMod.java 
> e2a638da518a2071ff15b8da6899646ec45c832a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMultiply.java 
> 99d1ad7f203d946fd89d26074bd0e00dec8b3a1a 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java 
> 4e45788936559bbb7cfe65e9ffd083747b37dcc2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPPlus.java 
> b1200e673e6b470b5fd1cc856270a6da615f16cb 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestUnaryMinus.java
>  ab6f6b79316818cac458390dc2d087091057c63b 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorArithmeticExpressions.java
>  02dec659ce421eef06f924bb6973070878d57be3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorMathFunctions.java
>  1950e92bd6d5e4f818588e691db30cd28193c716 
>   ql/src/test/queries/clientpositive/vectorization_numeric_overflows.q 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/vectorization_numeric_overflows.q.out 
> PRE-CREATION 
>   vector-code-gen/src/org/apache/hadoop/hive/tools/GenVectorCode.java 
>