> On April 10, 2016, 10:12 p.m., Jason Dere wrote: > > ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java, line > > 92 > > <https://reviews.apache.org/r/45697/diff/2/?file=1325411#file1325411line92> > > > > Since constant floating point values are in double, are there any big > > behavior changes here? > > > > - Comparisons (for example decimal_col = 12.5): I guess this means the > > comparison is done as double. I suspect this will affect ORC PPD because > > the column would need to be converted to double. Unless the constant value > > is converted to decimal constant (12.5BD). This could also be fixed by > > changing the Hive constant rules so that floating point constants are > > treated as Decimal types rather than double. > > > > - Does this affect IN clauses? I guess it's similar to comparison in > > that the comparison will now be done as doubles. > > > > - Table inserts from double into a decimal column will have an > > explicity cast to decimal, so that is ok. > > > > - I guess if there is an old-style UDF that only has a Decimal > > implementation, we would probably want to have a double version as well. I > > don't know if any such UDFs actually exist, if there was only one version > > of the UDF they usually have Double args. > > > > - Are there any other cases to think about that get affected by this > > change?
* yeah you are right since comparison will happen in double, casts need to be placed on decimal_col. However, for expression like double_col = 12.5BD after the patch there will be no implicit cast on column, so we will enable those patterns without cast. * Yeah for IN also if both double & decimal are present, than comparison will happen in double. However, I noticed that In uses getCommonClass() instead of getCommonClassForComparison(). That needs to be corrected. Also GenericUDFUtils::update() has a hack involving decimal/double type which should no longer be necessary adter this change. * Correct, inserts into decimal from double will need explicit cast. * One such I came across was UDFSign, there might be more. * This is all I can think of. - Ashutosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45697/#review128049 ----------------------------------------------------------- On April 5, 2016, 1:23 a.m., Ashutosh Chauhan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45697/ > ----------------------------------------------------------- > > (Updated April 5, 2016, 1:23 a.m.) > > > Review request for hive and Jason Dere. > > > Bugs: HIVE-13380 > https://issues.apache.org/jira/browse/HIVE-13380 > > > Repository: hive-git > > > Description > ------- > > Additionally, string should not be allowed to implictly cast to decimal since > there can be loss of precision there. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFSign.java 022b130 > ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java > 8488c21 > > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFOPMinus.java > 771a6c7 > > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFOPMultiply.java > 696682f > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFOPPlus.java > eba4894 > ql/src/test/queries/clientpositive/alter_partition_change_col.q 6861ca2 > ql/src/test/queries/clientpositive/alter_table_cascade.q 479fda4 > ql/src/test/results/clientpositive/perf/query32.q.out f9cfd69 > ql/src/test/results/clientpositive/perf/query65.q.out 37bb1b3 > ql/src/test/results/clientpositive/perf/query75.q.out 25a8776 > ql/src/test/results/clientpositive/perf/query89.q.out 0cda449 > ql/src/test/results/clientpositive/tez/vector_decimal_expressions.q.out > 2976cb5 > ql/src/test/results/clientpositive/udf_greatest.q.out 47cfb3f > ql/src/test/results/clientpositive/udf_least.q.out 2363abe > ql/src/test/results/clientpositive/vector_decimal_expressions.q.out 3ca326d > serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java > d3bb4e4 > > Diff: https://reviews.apache.org/r/45697/diff/ > > > Testing > ------- > > Regression suite > > > Thanks, > > Ashutosh Chauhan > >
