lidavidm commented on code in PR #41752:
URL: https://github.com/apache/arrow/pull/41752#discussion_r1609127038
##########
java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthViewVector.java:
##########
@@ -1345,7 +1366,29 @@ public void copyFrom(int fromIndex, int thisIndex,
ValueVector from) {
*/
@Override
public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
- throw new UnsupportedOperationException("copyFromSafe is not supported for
VariableWidthVector");
+ Preconditions.checkArgument(this.getMinorType() == from.getMinorType());
Review Comment:
We generally don't write explicit `this` outside of constructors
##########
java/vector/src/test/java/org/apache/arrow/vector/TestVarCharViewVector.java:
##########
@@ -1451,6 +1455,200 @@ public void
testSafeOverwriteLongFromALongerLongString() {
}
}
+ static Stream<Class<? extends BaseVariableWidthViewVector>> vectorProvider()
{
+ return Stream.of(
+ ViewVarCharVector.class,
+ ViewVarBinaryVector.class
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("vectorProvider")
+ public void testCopyFromWithNulls(Class<? extends
BaseVariableWidthViewVector> vectorClass)
+ throws NoSuchMethodException, InvocationTargetException,
InstantiationException, IllegalAccessException {
+ try (final BaseVariableWidthViewVector vector =
vectorClass.getConstructor(String.class, BufferAllocator.class)
+ .newInstance(EMPTY_SCHEMA_PATH, allocator);
+ final BaseVariableWidthViewVector vector2 =
vectorClass.getConstructor(String.class, BufferAllocator.class)
+ .newInstance(EMPTY_SCHEMA_PATH, allocator)) {
+ final int initialCapacity = 1024;
+ vector.setInitialCapacity(initialCapacity);
+ vector.allocateNew();
+ int capacity = vector.getValueCapacity();
+ assertTrue(capacity >= initialCapacity);
+
+ // setting number of values such that we have enough space in the
initial allocation
+ // to avoid re-allocation. This is to test copyFrom() without
re-allocation.
+ final int numberOfValues = initialCapacity / 2 /
ViewVarCharVector.ELEMENT_SIZE;
+
+ final String prefixString = generateRandomString(12);
+
+ for (int i = 0; i < numberOfValues; i++) {
+ if (i % 3 == 0) {
+ // null values
+ vector.setNull(i);
+ } else if (i % 3 == 1) {
+ // short strings
+ byte[] b = Integer.toString(i).getBytes(StandardCharsets.UTF_8);
+ vector.set(i, b, 0, b.length);
+ } else {
+ // long strings
+ byte[] b = (i + prefixString).getBytes(StandardCharsets.UTF_8);
+ vector.set(i, b, 0, b.length);
+ }
+ }
+
+ assertEquals(capacity, vector.getValueCapacity());
+
+ vector.setValueCount(numberOfValues);
+
+ for (int i = 0; i < numberOfValues; i++) {
+ if (i % 3 == 0) {
+ assertNull(vector.getObject(i));
+ } else if (i % 3 == 1) {
+
assertArrayEquals(Integer.toString(i).getBytes(StandardCharsets.UTF_8),
+ vector.get(i),
+ "unexpected value at index: " + i);
+ } else {
+ assertArrayEquals((i +
prefixString).getBytes(StandardCharsets.UTF_8),
+ vector.get(i),
+ "unexpected value at index: " + i);
+ }
+ }
+
+ vector2.setInitialCapacity(initialCapacity);
+ vector2.allocateNew();
+ int capacity2 = vector2.getValueCapacity();
+ assertEquals(capacity2, capacity);
+
+ for (int i = 0; i < numberOfValues; i++) {
+ vector2.copyFrom(i, i, vector);
+ if (i % 3 == 0) {
+ assertNull(vector2.getObject(i));
+ } else if (i % 3 == 1) {
+
assertArrayEquals(Integer.toString(i).getBytes(StandardCharsets.UTF_8),
+ vector.get(i),
+ "unexpected value at index: " + i);
+ } else {
+ assertArrayEquals((i +
prefixString).getBytes(StandardCharsets.UTF_8),
+ vector.get(i),
+ "unexpected value at index: " + i);
+ }
+ }
+
+ assertEquals(capacity, vector2.getValueCapacity());
+
+ vector2.setValueCount(numberOfValues);
+
+ for (int i = 0; i < numberOfValues; i++) {
+ if (i % 3 == 0) {
+ assertNull(vector2.getObject(i));
+ } else if (i % 3 == 1) {
+
assertArrayEquals(Integer.toString(i).getBytes(StandardCharsets.UTF_8),
+ vector.get(i),
+ "unexpected value at index: " + i);
+ } else {
+ assertArrayEquals((i +
prefixString).getBytes(StandardCharsets.UTF_8),
+ vector.get(i),
+ "unexpected value at index: " + i);
+ }
+ }
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource("vectorProvider")
+ public void testCopyFromSafeWithNulls(Class<? extends
BaseVariableWidthViewVector> vectorClass)
+ throws NoSuchMethodException, InvocationTargetException,
InstantiationException, IllegalAccessException {
+ try (final BaseVariableWidthViewVector vector =
vectorClass.getConstructor(String.class, BufferAllocator.class)
+ .newInstance(EMPTY_SCHEMA_PATH, allocator);
+ final BaseVariableWidthViewVector vector2 =
vectorClass.getConstructor(String.class, BufferAllocator.class)
+ .newInstance(EMPTY_SCHEMA_PATH, allocator)) {
Review Comment:
Can we avoid reflection and just parameterize with the instance instead of
the class?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
