----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11106/#review20870 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStart.java <https://reviews.apache.org/r/11106/#comment42976> setting isNull[0] to any value (true or false) is not necessary here because you set noNulls to true. ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStart.java <https://reviews.apache.org/r/11106/#comment42974> You should only copy n values (the current batch size), not inV.isNull.length. If n<<the default batch size then this code will be a performance problem. Also, you should only do this if !inV.noNulls. Move it inside the if. ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStartLen.java <https://reviews.apache.org/r/11106/#comment42980> this line is not needed because you set outV.noNulls to true ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStartLen.java <https://reviews.apache.org/r/11106/#comment42981> you should only do this if !noNulls. Move inside if statement. Also, only copy n values (batch.size). Not the full array. ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java <https://reviews.apache.org/r/11106/#comment42982> please add some comments to this function to say what the major sections are doing/testing ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java <https://reviews.apache.org/r/11106/#comment42984> explain why this assertion is correct ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java <https://reviews.apache.org/r/11106/#comment42985> explain assertion ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java <https://reviews.apache.org/r/11106/#comment42983> please add some comments to this function ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java <https://reviews.apache.org/r/11106/#comment42987> explain assertion ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java <https://reviews.apache.org/r/11106/#comment42988> explain - Eric Hanson On May 21, 2013, 5:56 p.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11106/ > ----------------------------------------------------------- > > (Updated May 21, 2013, 5:56 p.m.) > > > Review request for hive. > > > Description > ------- > > Add Vectorized Substr > > > This addresses bug HIVE-4495. > https://issues.apache.org/jira/browse/HIVE-4495 > > > Diffs > ----- > > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStart.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStartLen.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java > 6e26412 > > Diff: https://reviews.apache.org/r/11106/diff/ > > > Testing > ------- > > > Thanks, > > Timothy Chen > >
