----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40101/#review105902 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveGBOpConvUtil.java (line 1019) <https://reviews.apache.org/r/40101/#comment167622> Lines 1019-1026. We store the position of the argument for the function in the input (_argPos_). But then the logic is difficult to follow. _distinctPositions_ stores _argPos_, but _argPos_ is not used to obtain _rsUDAFParamColInfo_ from _rsColInfoLst_ in the _else_ block. Why? Does the code make the assumption that we visit the distinct positions in order i.e. pos 3 should always have been visited before 4? Thus, in the _else_ block, _gbInfo.gbKeys.size() + distinctPositions.size()_ = _argPos_, right? Then the _rsColInfoLst.get_ call could be taken out of the block. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveGBOpConvUtil.java (line 1043) <https://reviews.apache.org/r/40101/#comment167623> Can we take the call out of the _j_ for loop i.e. moved to the outer loop? ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveGBOpConvUtil.java (line 1058) <https://reviews.apache.org/r/40101/#comment167626> If the gby schema is --grpkey--,--distkey--,--values-- and we are trying to shift, would it be enough to sum up _distinctPositions.size_ and _argPos_ iff _argPos >= gbKeys.size()_, otherwise use _argPos_ ? ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveGBOpConvUtil.java (line 1069) <https://reviews.apache.org/r/40101/#comment167624> Can we take the call out of the _j_ for loop i.e. moved to the outer loop? - Jesús Camacho Rodríguez On Nov. 10, 2015, 12:58 a.m., pengcheng xiong wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40101/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2015, 12:58 a.m.) > > > Review request for hive and Jesús Camacho Rodríguez. > > > Repository: hive-git > > > Description > ------- > > The position in argList is mapped to a wrong column from RS operator > > > Diffs > ----- > > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveGBOpConvUtil.java > a129cf3 > ql/src/test/queries/clientpositive/cbo_rp_groupby3_noskew_multi_distinct.q > PRE-CREATION > ql/src/test/queries/clientpositive/cbo_rp_udf_percentile.q PRE-CREATION > ql/src/test/queries/clientpositive/cbo_rp_udf_percentile2.q PRE-CREATION > > ql/src/test/results/clientpositive/cbo_rp_groupby3_noskew_multi_distinct.q.out > PRE-CREATION > ql/src/test/results/clientpositive/cbo_rp_udf_percentile.q.out PRE-CREATION > ql/src/test/results/clientpositive/cbo_rp_udf_percentile2.q.out > PRE-CREATION > > Diff: https://reviews.apache.org/r/40101/diff/ > > > Testing > ------- > > > Thanks, > > pengcheng xiong > >
