----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33925/#review82796 -----------------------------------------------------------
exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java <https://reviews.apache.org/r/33925/#comment133615> Should this be startNewValue instead of startNewGroup? exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java <https://reviews.apache.org/r/33925/#comment133686> Are we retaining the parent/child nomenclature or are we going to use inner/outer. Seems that there is room for confusion. exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java <https://reviews.apache.org/r/33925/#comment133706> Throw a UserException here? exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java <https://reviews.apache.org/r/33925/#comment133808> Isn't the old implementation cleaner? There is no reason for a reader to be aware of the internal representation of a repeated vector, let alone manipulate it. exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseRepeatedValueVector.java <https://reviews.apache.org/r/33925/#comment133813> Add a comment explaining what size is supposed to return. exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseRepeatedValueVector.java <https://reviews.apache.org/r/33925/#comment133812> throw UserException here? exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedValueVector.java <https://reviews.apache.org/r/33925/#comment133817> Ideally, we should not expose this. Perhaps we can add a TODO (and JIRA) to eliminate the need to access the offeset vector. exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java <https://reviews.apache.org/r/33925/#comment133830> Should use the new UserException model with a message that can be displayed to the user. exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java <https://reviews.apache.org/r/33925/#comment133831> We should log a JIRA for this TODO RB won't let me see the diff, so this comment may be inaccurate. It appears that RepeatedVariableWidthVectorLike.java is not consistent with RepeatedFixedWidthVectorLike. The former is a Vector (extends ValueVector) while the latter is not a Vector (hence VectorLike). - Parth Chandra On May 7, 2015, 3:40 a.m., Hanifi Gunes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33925/ > ----------------------------------------------------------- > > (Updated May 7, 2015, 3:40 a.m.) > > > Review request for drill, Mehant Baid and Parth Chandra. > > > Bugs: DRILL-2150 > https://issues.apache.org/jira/browse/DRILL-2150 > > > Repository: drill-git > > > Description > ------- > > DRILL-2150: Create an abstraction for repeated value vectors. > > All value vectors > - Rely on getMetadataBuilder to iteratively construct vector metadata rather > than overriding getMetadata in each vector > - Fixed misc issues enhacing code re-use > - Removed generic template reference to concrete VV type from VV interface. > > ComplexWriters > - Fixed an off by one bug > > FixedValueVectors > - Fixed a value count bug > > RepeatedValueVectors > - Fixed wrong implementation of getValueCount. A value is whatever is stored > in the vector regardless if it is another vector or scalar type. The behavior > is consistent everywhere. > - Introduced getInnerValueCount and getInnerValueCountAt for values sitting > at second level to repeated types. > - All subclasses implement RepeatedVV interface > - Common functionality is abstracted out to base classes > > FlattenRB > - Refactored set flatten field block to make it more readable > - Fixed a wrong getValueCount call > > ProjectRB > - Fixed an issue regarding multiple projections not initializing nested > vectors. > > ZeroVector > - Used as initial vector value in repeated vectors instead of a "null" > reference > > ContainerVectorLike > - Common trait/mixin shared by nested vectors. > - Centralized logic to add vectors to nested types via use of VectorDescriptor > > > Diffs > ----- > > exec/java-exec/src/main/codegen/templates/ComplexReaders.java > fa1dac4ecce88e99b4c0b8cfcaec6095c6fed469 > exec/java-exec/src/main/codegen/templates/ComplexWriters.java > 49c75d132ac015931486c16913c327cc8f69678a > exec/java-exec/src/main/codegen/templates/FixedValueVectors.java > 6a924b762c99c0699dfb45eb50622a4b0e692a79 > exec/java-exec/src/main/codegen/templates/ListWriters.java > 6df4248d1fe527a3c2da86b880a13366f29aa5fb > exec/java-exec/src/main/codegen/templates/MapWriters.java > 6ee80351dbab5c16abd5ed2cc8fd40879a953afe > exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java > c06e29c3b1903016c1cd500ff3c5bebc47b8a1de > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java > 7a5b352b0b1210626cd19a62d63b73afdd756409 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenTemplate.java > 96209a23b3eb4f61dd9bc98a3dad53dc3938f5bd > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/Flattener.java > 2141ca2c47b19d66dc6cfce3fe088d7299c576ed > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java > 7b9fffb3a734f0063faec9f681b257ec98083da5 > exec/java-exec/src/main/java/org/apache/drill/exec/store/VectorHolder.java > 8387d49a61cb50c55863238fd9759daacb928cfc > > exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java > 7f8b6117570e21d84c952438807c51a52c900d92 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetRecordReader.java > 11d0042b5eef7977ba666100c0c5d66d69225266 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java > 7c77ca215b63a8dc3b921d0ada99c564f9fd81c1 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java > 0c6097c480a4ac0bdc6e37c16f0f7a643d1ca175 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseRepeatedValueVector.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java > 22f0fe7406139ac0bbe89fa51b1ff60591fb4df8 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/ContainerVectorLike.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java > eaae7ad6032f52ea3a2a718563096dc853bc1628 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedValueVector.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedVariableWidthVector.java > a49934168af1120b75cbc99911551f89bf56ea12 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedVector.java > df4279ac4566a55b9451cd2be6a7e57d3bcc4db8 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/SchemaChangeCallBack.java > 386ee34954b08f85e0ac2bc0db7e263566eb1ef6 > exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueVector.java > e4a0997ed12585ae27e06daf9bc242d9e1e8f6b6 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/VectorDescriptor.java > PRE-CREATION > exec/java-exec/src/main/java/org/apache/drill/exec/vector/ZeroVector.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java > 41388393c217973b9be910c69f15ee8e099f9d22 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java > c061029991f145751b686a9738a4021ec1cb12f6 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java > e5d48dd6fb07e7bd08b1b06e87ae24d839ffcaf2 > > exec/java-exec/src/test/java/org/apache/drill/exec/record/vector/TestValueVector.java > 1564aea4d41df3016b6b179162d0a0001bf62ca9 > > Diff: https://reviews.apache.org/r/33925/diff/ > > > Testing > ------- > > All tests pass. > > > Thanks, > > Hanifi Gunes > >
