> On Nov. 25, 2013, 11:27 p.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java,
> >  line 85
> > <https://reviews.apache.org/r/15777/diff/3/?file=389817#file389817line85>
> >
> >     Runtime exceptions represent problems that are the result of a 
> > programming problem, which seems proper for this case. That's, unless I 
> > programmed wrong, it shouldn't happen, which is exactly the case.
> >     
> >     I'm not sure if other type exception really buy us anything.

IllegalStateException provides better semantics. That is we are saying "an 
illegal state occurred" simply by the name which is more specific than simply 
"some kind of runtime error occured". I understand this is a small point but if 
there is a better semantic name for an exception, we should use it.


- Brock


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


On Nov. 22, 2013, 5:52 a.m., Xuefu Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15777/
> -----------------------------------------------------------
> 
> (Updated Nov. 22, 2013, 5:52 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5706
>     https://issues.apache.org/jira/browse/HIVE-5706
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> 1. Replaced the old implementations of power, ceil, floor, positive, and 
> negative with generic UDF implmentations.
> 2. Added unit tests for each.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 435d6e6 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
> 0a79256 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java 
> 151c648 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFCeil.java a01122e 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFFloor.java 3fdaf88 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFOPNegative.java bab1105 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFOPPositive.java ae11d74 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFPower.java 184c5d2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseUnary.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCeil.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFFloor.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFFloorCeilBase.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPPositive.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFPower.java 
> PRE-CREATION 
>   
> ql/src/test/org/apache/hadoop/hive/ql/exec/vector/TestVectorizationContext.java
>  73bcee0 
>   ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFCeil.java 
> PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFFloor.java 
> PRE-CREATION 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFOPNegative.java
>  PRE-CREATION 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFOPPositive.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFPower.java 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/decimal_udf.q.out ed5bc65 
>   ql/src/test/results/clientpositive/literal_decimal.q.out 78dac31 
>   ql/src/test/results/clientpositive/udf4.q.out 50db96c 
>   ql/src/test/results/clientpositive/udf7.q.out 7316449 
>   ql/src/test/results/clientpositive/vectorization_short_regress.q.out 
> c9296e1 
>   ql/src/test/results/clientpositive/vectorized_math_funcs.q.out 8bb0edf 
>   ql/src/test/results/compiler/plan/udf4.q.xml 145e244 
> 
> Diff: https://reviews.apache.org/r/15777/diff/
> 
> 
> Testing
> -------
> 
> All new tests and old test passed.
> 
> 
> Thanks,
> 
> Xuefu Zhang
> 
>

Reply via email to