----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29519/#review66783 -----------------------------------------------------------
First pass thoughts. Will do another review in the morning. common/src/main/java/org/apache/drill/common/collections/MapWithOrdinal.java <https://reviews.apache.org/r/29519/#comment110404> We need javadocs here to describe behaviors common/src/main/java/org/apache/drill/common/collections/MapWithOrdinal.java <https://reviews.apache.org/r/29519/#comment110403> this seems like it will cause weird behavior and unexpected changes in column ordering. it seems like we should have more linkedlist like behavior where each node gets and ordinal from the previous element in the list. exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java <https://reviews.apache.org/r/29519/#comment110399> Shouldn't these also be refactored out and shared with the other methods? exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java <https://reviews.apache.org/r/29519/#comment110400> You need to add Javadocs on these methods. You've made them behave in a clear way but you should doc so others don't have to review code to understand behavior. exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java <https://reviews.apache.org/r/29519/#comment110401> same as above. e.g. what happens on name conflict. exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/fn/TestJsonReaderWithSparseFiles.java <https://reviews.apache.org/r/29519/#comment110405> why did this change? - Jacques Nadeau On Dec. 31, 2014, 11:12 p.m., Hanifi Gunes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29519/ > ----------------------------------------------------------- > > (Updated Dec. 31, 2014, 11:12 p.m.) > > > Review request for drill, Jacques Nadeau, Parth Chandra, and Steven Phillips. > > > Bugs: DRILL-1885 > https://issues.apache.org/jira/browse/DRILL-1885 > > > Repository: drill-git > > > Description > ------- > > fix a problem regarding ordinal to vector mapping that report incorrect > result or fails a query. > fix failing unittest > refactor code, eliminate redundancy > > > Diffs > ----- > > > common/src/main/java/org/apache/drill/common/collections/MapWithOrdinal.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java > 23833b6 > > exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java > d50760a > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java > 1210d90 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java > cc3d24c > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java > 362d806 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java > e140c8b > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/VectorWithOrdinal.java > PRE-CREATION > > exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetComplex.java > 8405d0e > > exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/fn/TestJsonReaderWithSparseFiles.java > 7e4cf4b > > Diff: https://reviews.apache.org/r/29519/diff/ > > > Testing > ------- > > unit tests + all test suites pass. > > > Thanks, > > Hanifi Gunes > >
