----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11133/#review20538 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestCodeGen.java <https://reviews.apache.org/r/11133/#comment42356> missing apache license ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestCodeGen.java <https://reviews.apache.org/r/11133/#comment42357> classes need javadoc comment at beginning ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestCodeGen.java <https://reviews.apache.org/r/11133/#comment42354> See java coding style guide http://www.oracle.com/technetwork/java/codeconv-138413.html Differences for Hive: indent 2 chars not 4, and 100 char line length limit ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestCodeGen.java <https://reviews.apache.org/r/11133/#comment42355> need comment to explain purpose and contents of this matrix ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnColumnFilterVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42337> All in all this is a really nice approach. Nice clean test all on one page, fully templatized. ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnColumnFilterVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42332> BATCH_SIZE is optional in the constructors now and not normally recommended. The default is recommended. But that should not really matter here for correctness. ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnColumnFilterVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42334> Style issue with curly braces. Run ant checkstyle. ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnColumnFilterVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42336> selectedInUse can be true even of all rows have been filtered out If all rows qualify against a filter, an optimization is to set selectedInUse back to false. But this is not mandatory for correctness. ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnColumnOperationVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42341> you have quit a bit of extra trailing white space that shows up red in the review tool. I think you can set eclipse to get rid of it but I'm not sure how. ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnColumnOperationVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42342> style guide says put blank after comma ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnScalarFilterVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42344> what if input has noNulls? Then you should not look at isNull array. ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnScalarFilterVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42346> if the optimization is used whereby if every row qualifies, you set selectedInUse to false, this could give a false failure ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnScalarFilterVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42339> See earlier comment on this. ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnScalarOperationVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42351> test .txt templates need apache license ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnScalarOperationVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42350> scalarValue set but never used ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnScalarOperationVectorExpressionEvaluation.txt <https://reviews.apache.org/r/11133/#comment42347> not supposed to look at isNull if noNulls is set for vector ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/gen/TestColumnColumnFilterVectorExpressionEvaluation.java <https://reviews.apache.org/r/11133/#comment42353> Why not just used VectorizedRowBatch.DEFAULT_SIZE? is there a reason? - Eric Hanson On May 14, 2013, 12:34 a.m., tony murphy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11133/ > ----------------------------------------------------------- > > (Updated May 14, 2013, 12:34 a.m.) > > > Review request for hive, Jitendra Pandey, Eric Hanson, Sarvesh Sakalanaga, > and Remus Rusanu. > > > Description > ------- > > This patch adds Column Column, and Column Scalar vectorized execution tests. > These tests are generated in parallel with the vectorized expressions. The > tests focus is on validating the column vector and the vectorized row batch > metadata regarding nulls, repeating, and selection. > > Overview of Changes: > > CodeGen.java: > + joinPath, getCamelCaseType, readFile and writeFile made static for use in > TestCodeGen.java. > + filter types now specify null as their output type rather than "doesn't > matter" to make detection for test generation easier. > + support for test generation added. > > TestCodeGen.java & Templates: > TestClass.txt > TestColumnColumnFilterVectorExpressionEvaluation.txt, > TestColumnColumnOperationVectorExpressionEvaluation.txt, > TestColumnScalarFilterVectorExpressionEvaluation.txt, > TestColumnScalarOperationVectorExpressionEvaluation.txt > +This class is mutable and maintains a hashmap of TestSuiteClassName to test > cases. The tests cases are added over the course of vectorized expressions > class generation, with test classes being outputted at the end. For each > column vector (inputs and/or outputs) a matrix of pairwise covering Booleans > is used to generate test cases across nulls and repeating dimensions. Based > on the input column vector(s) nulls and repeating states the states of the > output column vector (if there is one) is validated, along with the null > vector. For filter operations the selection vector is validated against the > generated data. Each template corresponds to a class representing a test > suite. > > VectorizedRowGroupUtil.java > +added methods generateLongColumnVector and generateDoubleColumnVector for > generating the respective column vectors with optional nulls and/or repeating > values. > > > This addresses bug HIVE-4553. > https://issues.apache.org/jira/browse/HIVE-4553 > > > Diffs > ----- > > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/CodeGen.java > 53d9a7a > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestClass.txt > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestCodeGen.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnColumnFilterVectorExpressionEvaluation.txt > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnColumnOperationVectorExpressionEvaluation.txt > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnScalarFilterVectorExpressionEvaluation.txt > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/templates/TestColumnScalarOperationVectorExpressionEvaluation.txt > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/gen/TestColumnColumnFilterVectorExpressionEvaluation.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/gen/TestColumnColumnOperationVectorExpressionEvaluation.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/gen/TestColumnScalarFilterVectorExpressionEvaluation.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/gen/TestColumnScalarOperationVectorExpressionEvaluation.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/exec/vector/util/VectorizedRowGroupGenUtil.java > 8a07567 > > Diff: https://reviews.apache.org/r/11133/diff/ > > > Testing > ------- > > generated tests, and ran them. > > > Thanks, > > tony murphy > >
