> On Jan. 6, 2015, 4:32 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java, > > line 89 > > <https://reviews.apache.org/r/29519/diff/1/?file=804906#file804906line89> > > > > Shouldn't these also be refactored out and shared with the other > > methods?
This patch works around AbstractContainerVector and its subclasses. You are right that VectorContainer also needs some refactoring. I'd like to take a look at this on a sepearate issue though. > On Jan. 6, 2015, 4:32 a.m., Jacques Nadeau wrote: > > common/src/main/java/org/apache/drill/common/collections/MapWithOrdinal.java, > > line 33 > > <https://reviews.apache.org/r/29519/diff/1/?file=804904#file804904line33> > > > > We need javadocs here to describe behaviors done. > On Jan. 6, 2015, 4:32 a.m., Jacques Nadeau wrote: > > common/src/main/java/org/apache/drill/common/collections/MapWithOrdinal.java, > > line 85 > > <https://reviews.apache.org/r/29519/diff/1/?file=804904#file804904line85> > > > > 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. this state is meant to be internal. other pieces of code that used to rely on "implicit" ordering of vectors are now eliminated. ordinal assignment mechanizm is documented as well. should not be a problem. > On Jan. 6, 2015, 4:32 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java, > > line 75 > > <https://reviews.apache.org/r/29519/diff/1/?file=804907#file804907line75> > > > > 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. done. > On Jan. 6, 2015, 4:32 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java, > > line 114 > > <https://reviews.apache.org/r/29519/diff/1/?file=804907#file804907line114> > > > > same as above. e.g. what happens on name conflict. commented. > On Jan. 6, 2015, 4:32 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/fn/TestJsonReaderWithSparseFiles.java, > > line 123 > > <https://reviews.apache.org/r/29519/diff/1/?file=804913#file804913line123> > > > > why did this change? earlier impl didn't conform to field ordering in the schema due to the fact that vectors were assumed to be in order implicitly. now i do read list of fields from schema. so this test case got reverted to normal. - Hanifi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29519/#review66783 ----------------------------------------------------------- On Jan. 12, 2015, 11:06 p.m., Hanifi Gunes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29519/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2015, 11:06 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/AbstractMapVector.java > f126e5c > > 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/main/java/org/apache/drill/exec/vector/complex/impl/ComplexWriterImpl.java > 18b5e9e > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java > c60730c > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java > 15f8a2b > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleListReaderImpl.java > c2284ec > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java > 3ec66ff > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/VectorContainerWriter.java > 36184a7 > > 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 > > exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestRepeated.java > 3f125fa > > Diff: https://reviews.apache.org/r/29519/diff/ > > > Testing > ------- > > all test suites. > > > Thanks, > > Hanifi Gunes > >
