Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,7 @@ public void setSafe(int index, int isSet, int start, int end, ArrowBuf buffer) {
assert index >= 0;
final int dataLength = end - start;
fillEmpties(index);
handleSafe(index, end);
handleSafe(index, dataLength);
BitVectorHelper.setValidityBit(validityBuffer, index, isSet);
final int startOffset = offsetBuffer.getInt(index * OFFSET_WIDTH);
offsetBuffer.setInt((index + 1) * OFFSET_WIDTH, startOffset + dataLength);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1394,6 +1394,51 @@ public void testFillEmptiesNotOverfill() {
}
}

@Test
public void testSetSafeWithArrowBufNoExcessAllocs() {
final int numValues = BaseFixedWidthVector.INITIAL_VALUE_ALLOCATION * 2;
final byte[] valueBytes = "hello world".getBytes();
final int valueBytesLength = valueBytes.length;
final int isSet = 1;

try (
final VarCharVector fromVector = newVector(VarCharVector.class, EMPTY_SCHEMA_PATH,
MinorType.VARCHAR, allocator);
final VarCharVector toVector = newVector(VarCharVector.class, EMPTY_SCHEMA_PATH,
MinorType.VARCHAR, allocator)) {
/*
* Populate the from vector with 'numValues' with byte-arrays, each of size 'valueBytesLength'.
*/
fromVector.setInitialCapacity(numValues);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might make the unit test easier to read if you separated each section by a comment.
i.e.
//Setup

// Execute

// Verify

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've added comments.

fromVector.allocateNew();
for (int i = 0; i < numValues; ++i) {
fromVector.setSafe(i, valueBytes, 0 /*start*/, valueBytesLength);
}
fromVector.setValueCount(numValues);
ArrowBuf fromDataBuffer = fromVector.getDataBuffer();
assertTrue(numValues * valueBytesLength <= fromDataBuffer.capacity());

/*
* Copy the entries one-by-one from 'fromVector' to 'toVector', but use the setSafe with
* ArrowBuf API (instead of setSafe with byte-array).
*/
toVector.setInitialCapacity(numValues);
toVector.allocateNew();
for (int i = 0; i < numValues; i++) {
int start = fromVector.getstartOffset(i);
int end = fromVector.getstartOffset(i + 1);
toVector.setSafe(i, isSet, start, end, fromDataBuffer);
}

/*
* Since the 'fromVector' and 'toVector' have the same initial capacity, and were populated
* with the same varchar elements, the allocations and hence, the final capacity should be
* the same.
*/
assertEquals(fromDataBuffer.capacity(), toVector.getDataBuffer().capacity());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might pay to add a comment why the capacity is expected to be equal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added.

}
}

@Test
public void testCopyFromWithNulls() {
try (final VarCharVector vector = newVector(VarCharVector.class, EMPTY_SCHEMA_PATH, MinorType.VARCHAR, allocator);
Expand Down