Skip to content

Commit

Permalink
fix: updating documentation and clear usage
Browse files Browse the repository at this point in the history
  • Loading branch information
vibhatha committed Aug 13, 2024
1 parent 18670bc commit d8ad4e7
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 159 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,17 @@ public VectorWithOrdinal getChildVectorWithOrdinal(String name) {
return new VectorWithOrdinal(vector, ordinal);
}

/**
* Return the underlying buffers associated with this vector. Note that this doesn't impact the
* reference counts for this buffer, so it only should be used for in-context access. Also note
* that this buffer changes regularly, thus external classes shouldn't hold a reference to it
* (unless they change it).
*
* @param clear Whether to clear vector before returning, the buffers will still be refcounted but
* the returned array will be the only reference to them. Also, this won't clear the child
* buffers.
* @return The underlying {@link ArrowBuf buffers} that is used by this vector instance.
*/
@Override
public ArrowBuf[] getBuffers(boolean clear) {
final List<ArrowBuf> buffers = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,17 @@ public void reset() {
valueCount = 0;
}

/**
* Return the underlying buffers associated with this vector. Note that this doesn't impact the
* reference counts for this buffer, so it only should be used for in-context access. Also note
* that this buffer changes regularly, thus external classes shouldn't hold a reference to it
* (unless they change it).
*
* @param clear Whether to clear vector before returning, the buffers will still be refcounted but
* the returned array will be the only reference to them. Also, this won't clear the child
* buffers.
* @return The underlying {@link ArrowBuf buffers} that is used by this vector instance.
*/
@Override
public ArrowBuf[] getBuffers(boolean clear) {
final ArrowBuf[] buffers;
Expand All @@ -279,7 +290,7 @@ public ArrowBuf[] getBuffers(boolean clear) {
} else {
List<ArrowBuf> list = new ArrayList<>();
list.add(offsetBuffer);
list.addAll(Arrays.asList(vector.getBuffers(clear)));
list.addAll(Arrays.asList(vector.getBuffers(false)));
buffers = list.toArray(new ArrowBuf[list.size()]);
}
if (clear) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -882,12 +882,13 @@ public void reset() {

/**
* Return the underlying buffers associated with this vector. Note that this doesn't impact the
* reference counts for this buffer so it only should be used for in-context access. Also note
* that this buffer changes regularly thus external classes shouldn't hold a reference to it
* reference counts for this buffer, so it only should be used for in-context access. Also note
* that this buffer changes regularly, thus external classes shouldn't hold a reference to it
* (unless they change it).
*
* @param clear Whether to clear vector before returning; the buffers will still be refcounted but
* the returned array will be the only reference to them
* @param clear Whether to clear vector before returning, the buffers will still be refcounted but
* the returned array will be the only reference to them. Also, this won't clear the child
* buffers.
* @return The underlying {@link ArrowBuf buffers} that is used by this vector instance.
*/
@Override
Expand All @@ -900,7 +901,7 @@ public ArrowBuf[] getBuffers(boolean clear) {
List<ArrowBuf> list = new ArrayList<>();
list.add(offsetBuffer);
list.add(validityBuffer);
list.addAll(Arrays.asList(vector.getBuffers(clear)));
list.addAll(Arrays.asList(vector.getBuffers(false)));
buffers = list.toArray(new ArrowBuf[list.size()]);
}
if (clear) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,8 @@ public void reset() {
* (unless they change it).
*
* @param clear Whether to clear vector before returning, the buffers will still be refcounted but
* the returned array will be the only reference to them
* the returned array will be the only reference to them. Also, this won't clear the child
* buffers.
* @return The underlying {@link ArrowBuf buffers} that is used by this vector instance.
*/
@Override
Expand All @@ -561,7 +562,7 @@ public ArrowBuf[] getBuffers(boolean clear) {
list.add(validityBuffer);
list.add(offsetBuffer);
list.add(sizeBuffer);
list.addAll(Arrays.asList(vector.getBuffers(clear)));
list.addAll(Arrays.asList(vector.getBuffers(false)));
buffers = list.toArray(new ArrowBuf[list.size()]);
}
if (clear) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -726,12 +726,13 @@ public void reset() {

/**
* Return the underlying buffers associated with this vector. Note that this doesn't impact the
* reference counts for this buffer so it only should be used for in-context access. Also note
* that this buffer changes regularly thus external classes shouldn't hold a reference to it
* reference counts for this buffer, so it only should be used for in-context access. Also note
* that this buffer changes regularly, thus external classes shouldn't hold a reference to it
* (unless they change it).
*
* @param clear Whether to clear vector before returning; the buffers will still be refcounted but
* the returned array will be the only reference to them
* @param clear Whether to clear vector before returning, the buffers will still be refcounted but
* the returned array will be the only reference to them. Also, this won't clear the child
* buffers.
* @return The underlying {@link ArrowBuf buffers} that is used by this vector instance.
*/
@Override
Expand All @@ -744,7 +745,7 @@ public ArrowBuf[] getBuffers(boolean clear) {
List<ArrowBuf> list = new ArrayList<>();
list.add(offsetBuffer);
list.add(validityBuffer);
list.addAll(Arrays.asList(vector.getBuffers(clear)));
list.addAll(Arrays.asList(vector.getBuffers(false)));
buffers = list.toArray(new ArrowBuf[list.size()]);
}
if (clear) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,8 @@ public void reset() {
* (unless they change it).
*
* @param clear Whether to clear vector before returning, the buffers will still be refcounted but
* the returned array will be the only reference to them
* the returned array will be the only reference to them. Also, this won't clear the child
* buffers.
* @return The underlying {@link ArrowBuf buffers} that is used by this vector instance.
*/
@Override
Expand All @@ -719,7 +720,7 @@ public ArrowBuf[] getBuffers(boolean clear) {
list.add(validityBuffer);
list.add(offsetBuffer);
list.add(sizeBuffer);
list.addAll(Arrays.asList(vector.getBuffers(clear)));
list.addAll(Arrays.asList(vector.getBuffers(false)));
buffers = list.toArray(new ArrowBuf[list.size()]);
}
if (clear) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,12 +396,13 @@ public int getValueCapacity() {

/**
* Return the underlying buffers associated with this vector. Note that this doesn't impact the
* reference counts for this buffer so it only should be used for in-context access. Also note
* that this buffer changes regularly thus external classes shouldn't hold a reference to it
* reference counts for this buffer, so it only should be used for in-context access. Also note
* that this buffer changes regularly, thus external classes shouldn't hold a reference to it
* (unless they change it).
*
* @param clear Whether to clear vector before returning; the buffers will still be refcounted but
* the returned array will be the only reference to them
* @param clear Whether to clear vector before returning, the buffers will still be refcounted but
* the returned array will be the only reference to them. Also, this won't clear the child
* buffers.
* @return The underlying {@link ArrowBuf buffers} that is used by this vector instance.
*/
@Override
Expand All @@ -413,7 +414,7 @@ public ArrowBuf[] getBuffers(boolean clear) {
} else {
List<ArrowBuf> list = new ArrayList<>();
list.add(validityBuffer);
list.addAll(Arrays.asList(super.getBuffers(clear)));
list.addAll(Arrays.asList(super.getBuffers(false)));
buffers = list.toArray(new ArrowBuf[list.size()]);
}
if (clear) {
Expand Down
139 changes: 0 additions & 139 deletions java/vector/src/test/java/org/apache/arrow/vector/TestVectorReset.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,25 +64,6 @@ private void resetVectorAndVerify(ValueVector vector, ArrowBuf[] bufs) {
assertEquals(0, vector.getValueCount());
}

private void resetClearVectorAndVerify(ValueVector vector, ArrowBuf[] bufs) {
long[] sizeBefore = new long[bufs.length];
for (int i = 0; i < bufs.length; i++) {
sizeBefore[i] = bufs[i].capacity();
}
vector.reset();
for (int i = 0; i < bufs.length; i++) {
assertEquals(sizeBefore[i], bufs[i].capacity());
}
assertEquals(0, vector.getValueCount());
// clear the retrieved buffers
for (ArrowBuf buf : bufs) {
// clear the buffer to release the memory
while (buf.refCnt() > 0) {
buf.close();
}
}
}

private void verifyBufferZeroed(ArrowBuf buf) {
for (int i = 0; i < buf.capacity(); i++) {
assertTrue((byte) 0 == buf.getByte(i));
Expand All @@ -99,16 +80,6 @@ public void testFixedTypeReset() {
}
}

@Test
public void testFixedTypeResetAndClear() {
try (final UInt4Vector vector = new UInt4Vector("UInt4", allocator)) {
vector.allocateNewSafe();
vector.setNull(0);
vector.setValueCount(1);
resetClearVectorAndVerify(vector, vector.getBuffers(true));
}
}

@Test
public void testVariableTypeReset() {
try (final VarCharVector vector = new VarCharVector("VarChar", allocator)) {
Expand All @@ -121,18 +92,6 @@ public void testVariableTypeReset() {
}
}

@Test
public void testVariableTypeResetAndClear() {
try (final VarCharVector vector = new VarCharVector("VarChar", allocator)) {
vector.allocateNewSafe();
vector.set(0, "a".getBytes(StandardCharsets.UTF_8));
vector.setLastSet(0);
vector.setValueCount(1);
resetClearVectorAndVerify(vector, vector.getBuffers(true));
assertEquals(-1, vector.getLastSet());
}
}

@Test
public void testVariableViewTypeReset() {
try (final ViewVarCharVector vector = new ViewVarCharVector("ViewVarChar", allocator)) {
Expand All @@ -145,18 +104,6 @@ public void testVariableViewTypeReset() {
}
}

@Test
public void testVariableViewTypeResetAndClear() {
try (final ViewVarCharVector vector = new ViewVarCharVector("ViewVarChar", allocator)) {
vector.allocateNewSafe();
vector.set(0, "a".getBytes(StandardCharsets.UTF_8));
vector.setLastSet(0);
vector.setValueCount(1);
resetClearVectorAndVerify(vector, vector.getBuffers(true));
assertEquals(-1, vector.getLastSet());
}
}

@Test
public void testLargeVariableTypeReset() {
try (final LargeVarCharVector vector = new LargeVarCharVector("LargeVarChar", allocator)) {
Expand All @@ -169,18 +116,6 @@ public void testLargeVariableTypeReset() {
}
}

@Test
public void testLargeVariableTypeResetAndClear() {
try (final LargeVarCharVector vector = new LargeVarCharVector("LargeVarChar", allocator)) {
vector.allocateNewSafe();
vector.set(0, "a".getBytes(StandardCharsets.UTF_8));
vector.setLastSet(0);
vector.setValueCount(1);
resetClearVectorAndVerify(vector, vector.getBuffers(true));
assertEquals(-1, vector.getLastSet());
}
}

@Test
public void testListTypeReset() {
try (final ListVector variableList =
Expand Down Expand Up @@ -215,40 +150,6 @@ public void testListTypeReset() {
}
}

@Test
public void testListTypeResetAndClear() {
try (final ListVector variableList =
new ListVector(
"VarList", allocator, FieldType.nullable(MinorType.INT.getType()), null);
final FixedSizeListVector fixedList =
new FixedSizeListVector(
"FixedList", allocator, FieldType.nullable(new FixedSizeList(2)), null);
final ListViewVector variableViewList =
new ListViewVector(
"VarListView", allocator, FieldType.nullable(MinorType.INT.getType()), null)) {
// ListVector
variableList.allocateNewSafe();
variableList.startNewValue(0);
variableList.endValue(0, 0);
variableList.setValueCount(1);
resetClearVectorAndVerify(variableList, variableList.getBuffers(true));
assertEquals(-1, variableList.getLastSet());

// FixedSizeListVector
fixedList.allocateNewSafe();
fixedList.setNull(0);
fixedList.setValueCount(1);
resetClearVectorAndVerify(fixedList, fixedList.getBuffers(true));

// ListViewVector
variableViewList.allocateNewSafe();
variableViewList.startNewValue(0);
variableViewList.endValue(0, 0);
variableViewList.setValueCount(1);
resetClearVectorAndVerify(variableViewList, variableViewList.getBuffers(true));
}
}

@Test
public void testStructTypeReset() {
try (final NonNullableStructVector nonNullableStructVector =
Expand All @@ -274,31 +175,6 @@ public void testStructTypeReset() {
}
}

@Test
public void testStructTypeResetAndClear() {
try (final NonNullableStructVector nonNullableStructVector =
new NonNullableStructVector(
"Struct", allocator, FieldType.nullable(MinorType.INT.getType()), null);
final StructVector structVector =
new StructVector(
"NullableStruct", allocator, FieldType.nullable(MinorType.INT.getType()), null)) {
// NonNullableStructVector
nonNullableStructVector.allocateNewSafe();
IntVector structChild =
nonNullableStructVector.addOrGet(
"child", FieldType.nullable(new Int(32, true)), IntVector.class);
structChild.setNull(0);
nonNullableStructVector.setValueCount(1);
resetClearVectorAndVerify(nonNullableStructVector, nonNullableStructVector.getBuffers(true));

// StructVector
structVector.allocateNewSafe();
structVector.setNull(0);
structVector.setValueCount(1);
resetClearVectorAndVerify(structVector, structVector.getBuffers(true));
}
}

@Test
public void testUnionTypeReset() {
try (final UnionVector vector =
Expand All @@ -313,19 +189,4 @@ public void testUnionTypeReset() {
resetVectorAndVerify(vector, vector.getBuffers(false));
}
}

@Test
public void testUnionTypeResetAndClear() {
try (final UnionVector vector =
new UnionVector("Union", allocator, /* field type */ null, /* call-back */ null);
final IntVector dataVector = new IntVector("Int", allocator)) {
vector.getBufferSize();
vector.allocateNewSafe();
dataVector.allocateNewSafe();
vector.addVector(dataVector);
dataVector.setNull(0);
vector.setValueCount(1);
resetClearVectorAndVerify(vector, vector.getBuffers(true));
}
}
}

0 comments on commit d8ad4e7

Please sign in to comment.