> On Oct. 21, 2018, 8:55 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java > > Line 1171 (original), 1176 (patched) > > <https://reviews.apache.org/r/69019/diff/2/?file=2101638#file2101638line1190> > > > > Can remove this TODO now.
this comment applies to other comparisions as well <,<=,> ... etc I feel that changing that may cause other issues....and this patch is already trying to handle 2-3 correctness issues of IN > On Oct. 21, 2018, 8:55 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java > > Lines 1386-1397 (patched) > > <https://reviews.apache.org/r/69019/diff/2/?file=2101638#file2101638line1404> > > > > Why we need to add this logic now? Prior to this patch, we would have > > coerced RHS constant to decimal if column is of type decimal. > > > > AFAICS, this if() can only be executed if we had in query either > > float_col = 4.2BD or the expression is dobule_col = 4.2BD but that will be > > handled by if > > (PrimitiveObjectInspectorUtils.floatTypeEntry.equals(primitiveTypeEntry)) { > > else if > > (PrimitiveObjectInspectorUtils.doubleTypeEntry.equals(primitiveTypeEntry)) this is needed because for structs I think this was not handled. With the earlier unwinding of IN this might not be needed anymore > On Oct. 21, 2018, 8:55 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java > > Line 1298 (original), 1404 (patched) > > <https://reviews.apache.org/r/69019/diff/2/?file=2101638#file2101638line1423> > > > > My suggestion is to do final HiveChar newValue = new > > HiveChar(constValue, constValue.getlength()); I was afraid that it will cause problems...changing it caused some regessions to me. I propose to keep it right now as is...and file a followup ticket to address this and the comparision returning null things. - Zoltan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69019/#review209831 ----------------------------------------------------------- On Oct. 20, 2018, 7:51 a.m., Zoltan Haindrich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69019/ > ----------------------------------------------------------- > > (Updated Oct. 20, 2018, 7:51 a.m.) > > > Review request for hive, Ashutosh Chauhan and Vineet Garg. > > > Bugs: HIVE-20617 > https://issues.apache.org/jira/browse/HIVE-20617 > > > Repository: hive-git > > > Description > ------- > > For IN expressions the types were never corrected; and pointlookupoptimizer > was probably leaving behind fields already which were uncomparable; > HIVE-20296 exposed it further by changing the minimal number from 32 to 2. > > This change generalizes the retyping of constants to also run it for the IN > operator ; and also for struct-s. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/type/HiveChar.java 29dc06dca1 > ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java e7d71595c7 > ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java > 4968d16876 > ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java > c274fd7cc9 > ql/src/test/queries/clientpositive/in_typecheck_char.q PRE-CREATION > ql/src/test/queries/clientpositive/in_typecheck_pointlook.q PRE-CREATION > ql/src/test/queries/clientpositive/in_typecheck_varchar.q PRE-CREATION > ql/src/test/results/clientpositive/alter_partition_coltype.q.out f6c3c5642e > ql/src/test/results/clientpositive/in_typecheck2.q.out PRE-CREATION > ql/src/test/results/clientpositive/in_typecheck_char.q.out PRE-CREATION > ql/src/test/results/clientpositive/in_typecheck_pointlook.q.out > PRE-CREATION > ql/src/test/results/clientpositive/in_typecheck_varchar.q.out PRE-CREATION > ql/src/test/results/clientpositive/infer_const_type.q.out e1d7de5422 > ql/src/test/results/clientpositive/join45.q.out 47aaf7d0ab > ql/src/test/results/clientpositive/join47.q.out 4d9e937815 > ql/src/test/results/clientpositive/llap/dec_str.q.out 554031e952 > ql/src/test/results/clientpositive/llap/explainuser_1.q.out f240468558 > ql/src/test/results/clientpositive/llap/lineage3.q.out cf38816127 > ql/src/test/results/clientpositive/llap/vectorization_13.q.out 4ce654f960 > ql/src/test/results/clientpositive/llap/vectorization_6.q.out a2f730beca > ql/src/test/results/clientpositive/llap/vectorization_8.q.out 21ce7b8ebd > ql/src/test/results/clientpositive/llap/vectorization_short_regress.q.out > 7f1c6a295e > ql/src/test/results/clientpositive/mapjoin47.q.out 294dd69de5 > ql/src/test/results/clientpositive/parquet_vectorization_13.q.out > 0efce98b55 > ql/src/test/results/clientpositive/parquet_vectorization_6.q.out 0bb6888364 > ql/src/test/results/clientpositive/parquet_vectorization_8.q.out 957bd7b264 > ql/src/test/results/clientpositive/ppd_udf_col.q.out 814fb5afcf > ql/src/test/results/clientpositive/spark/cbo_simple_select.q.out acf91bf178 > ql/src/test/results/clientpositive/spark/parquet_vectorization_13.q.out > 3812239343 > ql/src/test/results/clientpositive/spark/parquet_vectorization_6.q.out > 6108457aad > ql/src/test/results/clientpositive/spark/parquet_vectorization_8.q.out > 3352dedc58 > ql/src/test/results/clientpositive/spark/spark_explainuser_1.q.out > f5a4c9ad86 > ql/src/test/results/clientpositive/spark/subquery_scalar.q.out b3252f5415 > ql/src/test/results/clientpositive/spark/vectorization_13.q.out 34ec9c42dd > ql/src/test/results/clientpositive/spark/vectorization_6.q.out 5679bb8cfa > ql/src/test/results/clientpositive/spark/vectorization_short_regress.q.out > 231dea6de3 > ql/src/test/results/clientpositive/vectorization_13.q.out 8897f8427f > ql/src/test/results/clientpositive/vectorization_6.q.out 8dedb63e7d > ql/src/test/results/clientpositive/vectorization_8.q.out d81df76a2f > > > Diff: https://reviews.apache.org/r/69019/diff/2/ > > > Testing > ------- > > > Thanks, > > Zoltan Haindrich > >
