> On April 10, 2015, 5:42 p.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java, > > line 222 > > <https://reviews.apache.org/r/33035/diff/1/?file=921990#file921990line222> > > > > I thought this needed to call buf.retain() unconditionally (not just if > > (clear)), since we're adding references by putting these into the resulting > > array. I also changed the comment in BaseDataVector.getBuffers() about this > > -- that claims that no new references are added, but that's wrong.
That's not needed. As buffers are not released yet we can hand over without retaining. Problems may surface when getBuffers(false) is called. Once this call is made both the vector and consumer operate on the same set of buffers, which sounds unsafe. - Hanifi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33035/#review79714 ----------------------------------------------------------- On April 13, 2015, 10:07 p.m., Hanifi Gunes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33035/ > ----------------------------------------------------------- > > (Updated April 13, 2015, 10:07 p.m.) > > > Review request for drill, Mehant Baid and Parth Chandra. > > > Repository: drill-git > > > Description > ------- > > DRILL-2719: ValueVector#getBuffers(clear) must consistently clear vectors & > retain buffers > > BaseDataValueVector > - getBuffers now rely on getBufferSize while determining buffers to return > - getBuffers maintains reference count to underlying buffers while clearing > the vector > - getBufferSize relies on value count reported by accessor while determining > buffer size > - replaced DeadBuf references with an empty buffer. underlying buffer now > should never be *null*. > > Templates & VV subtypes > - ensure getBuffers conforms to VV#getBuffers > > TestEmptyPopulator > - make mock allocator return an empty buffer when requested > > > Diffs > ----- > > exec/java-exec/src/main/codegen/templates/NullableValueVectors.java > 075316e4f3ac5327e7893688c5e88cfee98e50bc > exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java > c7cf8e6fe18f1b9813ae22495ac79a447f61cfff > exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java > edb851eb10be43d889ce5fd98d9bde036707870a > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java > d48ea99237bb822cafc8b835c3af0f4789c6eb29 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java > b0783afe57317dcd8dc7a2a8d967dcdb1f305edb > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java > c0f529961343145d67f835a95f58c4eaf2fae2a4 > > exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/TestEmptyPopulator.java > 8426a6abbf0c20be6f81bc82521f31ed7cde2557 > > Diff: https://reviews.apache.org/r/33035/diff/ > > > Testing > ------- > > unit and beyond > > > Thanks, > > Hanifi Gunes > >
