----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19718/#review38752 -----------------------------------------------------------
Looks good overall. Only minor comments. ql/src/gen/vectorization/ExpressionTemplates/FilterDecimalColumnBetween.txt <https://reviews.apache.org/r/19718/#comment71027> please remove all trailing whitespace in this file ql/src/gen/vectorization/ExpressionTemplates/FilterDecimalColumnBetween.txt <https://reviews.apache.org/r/19718/#comment71034> add blank after // ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java <https://reviews.apache.org/r/19718/#comment71038> Couldn't determine common type ... sounds better ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/DecimalColumnInList.java <https://reviews.apache.org/r/19718/#comment71053> Change comment. This is not a filter, it is a Boolean-valued expression. ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/DecimalColumnInList.java <https://reviews.apache.org/r/19718/#comment71052> Remove the comment about "This is optimized for lookup of the data type of the column." because that doesn't apply here since you're using the standard HashSet. But it is still pretty good :-) ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/DecimalColumnInList.java <https://reviews.apache.org/r/19718/#comment71057> formatting: j=0 ==> j = 0 ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/DecimalColumnInList.java <https://reviews.apache.org/r/19718/#comment71059> add blanks line before comment and space after // ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/FilterDecimalColumnInList.java <https://reviews.apache.org/r/19718/#comment71062> remove "This is optimized...." ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/FilterDecimalColumnInList.java <https://reviews.apache.org/r/19718/#comment71061> see formatting comments for DecimalColumnInList - Eric Hanson On March 27, 2014, 7:02 a.m., Jitendra Pandey wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19718/ > ----------------------------------------------------------- > > (Updated March 27, 2014, 7:02 a.m.) > > > Review request for hive and Eric Hanson. > > > Bugs: HIVE-6752 > https://issues.apache.org/jira/browse/HIVE-6752 > > > Repository: hive-git > > > Description > ------- > > Vectorized Between and IN expressions don't work with decimal, date types. > > > Diffs > ----- > > ant/src/org/apache/hadoop/hive/ant/GenVectorCode.java 44b0c59 > ql/src/gen/vectorization/ExpressionTemplates/FilterDecimalColumnBetween.txt > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java > 96e74a9 > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastDateToString.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/DecimalColumnInList.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/FilterDecimalColumnInList.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/IDecimalInExpr.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java > c2240c0 > ql/src/test/queries/clientpositive/vector_between_in.q PRE-CREATION > ql/src/test/results/clientpositive/vector_between_in.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/19718/diff/ > > > Testing > ------- > > > Thanks, > > Jitendra Pandey > >
