Skip to content

Commit

Permalink
perform self-review round 1
Browse files Browse the repository at this point in the history
  • Loading branch information
llama90 committed Jun 7, 2024
1 parent db131be commit d8d4af3
Show file tree
Hide file tree
Showing 17 changed files with 70 additions and 118 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ public void testCopyFromWithNulls() {
if (i % 3 == 0) {
assertNull(vector.getObject(i));
} else {
assertEquals(Integer.toString(i), vector.getObject(i).toString(),
"unexpected value at index: " + i);
assertEquals(Integer.toString(i), vector.getObject(i).toString(), "unexpected value at index: " + i);
}
}

Expand All @@ -115,8 +114,7 @@ public void testCopyFromWithNulls() {
if (i % 3 == 0) {
assertNull(vector2.getObject(i));
} else {
assertEquals(Integer.toString(i), vector2.getObject(i).toString(),
"unexpected value at index: " + i);
assertEquals(Integer.toString(i), vector2.getObject(i).toString(), "unexpected value at index: " + i);
}
}

Expand All @@ -130,8 +128,7 @@ public void testCopyFromWithNulls() {
if (i % 3 == 0) {
assertNull(vector2.getObject(i));
} else {
assertEquals(Integer.toString(i), vector2.getObject(i).toString(),
"unexpected value at index: " + i);
assertEquals(Integer.toString(i), vector2.getObject(i).toString(), "unexpected value at index: " + i);
}
}
}
Expand Down Expand Up @@ -501,8 +498,7 @@ public void testCopyFromWithNulls6() {
if ((i & 1) == 0) {
assertNull(vector1.getObject(i));
} else {
assertEquals(123456.7865 + (double) i, vector1.get(i), 0,
"unexpected value at index: " + i);
assertEquals(123456.7865 + (double) i, vector1.get(i), 0, "unexpected value at index: " + i);
}
}

Expand All @@ -529,10 +525,7 @@ public void testCopyFromWithNulls6() {
if (((i & 1) == 0) || (i >= initialCapacity)) {
assertNull(vector2.getObject(i));
} else {
assertEquals(
123456.7865 + (double) i,
vector2.get(i), 0,
"unexpected value at index: " + i);
assertEquals(123456.7865 + (double) i, vector2.get(i), 0, "unexpected value at index: " + i);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ public void testDecimal256DifferentScaleAndPrecision() {
BigDecimal decimal = new BigDecimal(BigInteger.valueOf(12345), 2);
UnsupportedOperationException ue =
assertThrows(UnsupportedOperationException.class, () -> decimalVector.setSafe(0, decimal));
assertEquals("BigDecimal precision cannot be greater than that in the Arrow vector: 5 > 4", ue
.getMessage());
assertEquals("BigDecimal precision cannot be greater than that in the Arrow vector: 5 > 4", ue.getMessage());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ public void testSetOffset() {

duv.setValueCount(4);

assertEquals(duv.getObject(0), 42);
assertEquals(duv.getObject(1), 43);
assertEquals(duv.getObject(2), 3.14);
assertEquals(duv.getObject(3), 44);
assertEquals(42, duv.getObject(0));
assertEquals(43, duv.getObject(1));
assertEquals(3.14, duv.getObject(2));
assertEquals(44, duv.getObject(3));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -944,8 +944,7 @@ public void testWriterUsingHolderGetTimestampMilliTZField() {
holder.value = 77777;
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
() -> writer.timeStampMilliTZ().write(holder));
assertEquals("holder.timezone: AsdfTimeZone not equal to vector timezone: SomeFakeTimeZone",
ex.getMessage());
assertEquals("holder.timezone: AsdfTimeZone not equal to vector timezone: SomeFakeTimeZone", ex.getMessage());

writer.endList();
vector.setValueCount(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ public void testSplitAndTransfer() throws Exception {
/* check the toVector output after doing the splitAndTransfer */
for (int i = 0; i < length; i++) {
assertEquals(sourceVector.getObject(start + i), toVector.getObject(i),
"Different data at indexes: " + (start + i) + "and " + i);
"Different data at indexes: " + (start + i) + "and " + i);
}
}
}
Expand Down Expand Up @@ -505,23 +505,19 @@ public void testCreateNewVectorWithoutTypeExceptionThrown() {
new UnionVector(EMPTY_SCHEMA_PATH, allocator, /* field type */ null, /* call-back */ null)) {
IllegalArgumentException e1 = assertThrows(IllegalArgumentException.class,
() -> vector.getTimeStampMilliTZVector());
assertEquals("No TimeStampMilliTZ present. Provide ArrowType argument to create a new vector",
e1.getMessage());
assertEquals("No TimeStampMilliTZ present. Provide ArrowType argument to create a new vector", e1.getMessage());

IllegalArgumentException e2 = assertThrows(IllegalArgumentException.class,
() -> vector.getDurationVector());
assertEquals("No Duration present. Provide ArrowType argument to create a new vector",
e2.getMessage());
assertEquals("No Duration present. Provide ArrowType argument to create a new vector", e2.getMessage());

IllegalArgumentException e3 = assertThrows(IllegalArgumentException.class,
() -> vector.getFixedSizeBinaryVector());
assertEquals("No FixedSizeBinary present. Provide ArrowType argument to create a new vector",
e3.getMessage());
assertEquals("No FixedSizeBinary present. Provide ArrowType argument to create a new vector", e3.getMessage());

IllegalArgumentException e4 = assertThrows(IllegalArgumentException.class,
() -> vector.getDecimalVector());
assertEquals("No Decimal present. Provide ArrowType argument to create a new vector",
e4.getMessage());
assertEquals("No Decimal present. Provide ArrowType argument to create a new vector", e4.getMessage());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2142,8 +2142,7 @@ public void testVectorLoadUnload() {

try (
ArrowRecordBatch recordBatch = vectorUnloader.getRecordBatch();
BufferAllocator finalVectorsAllocator = allocator.newChildAllocator("new vector", 0,
Long.MAX_VALUE);
BufferAllocator finalVectorsAllocator = allocator.newChildAllocator("new vector", 0, Long.MAX_VALUE);
VectorSchemaRoot schemaRoot2 = VectorSchemaRoot.create(schema, finalVectorsAllocator);
) {

Expand Down Expand Up @@ -2382,14 +2381,12 @@ public void testSetInitialCapacity() {
vector.setInitialCapacity(defaultCapacity, 0.1);
vector.allocateNew();
assertEquals(defaultCapacity, vector.getValueCapacity());
assertEquals(CommonUtil.nextPowerOfTwo((int) (defaultCapacity * 0.1)), vector.getDataBuffer()
.capacity());
assertEquals(CommonUtil.nextPowerOfTwo((int) (defaultCapacity * 0.1)), vector.getDataBuffer().capacity());

vector.setInitialCapacity(defaultCapacity, 0.01);
vector.allocateNew();
assertEquals(defaultCapacity, vector.getValueCapacity());
assertEquals(CommonUtil.nextPowerOfTwo((int) (defaultCapacity * 0.01)), vector.getDataBuffer()
.capacity());
assertEquals(CommonUtil.nextPowerOfTwo((int) (defaultCapacity * 0.01)), vector.getDataBuffer().capacity());

vector.setInitialCapacity(5, 0.01);
vector.allocateNew();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,7 @@ public void testVarCharAllocateNew() throws Exception {

// verify that the validity buffer and value buffer have capacity for at least 'count' elements.
assertTrue(vector.getValidityBuffer().capacity() >= DataSizeRoundingUtil.divideBy8Ceil(count));
assertTrue(vector.getOffsetBuffer().capacity() >= (count + 1) * BaseVariableWidthVector
.OFFSET_WIDTH);
assertTrue(vector.getOffsetBuffer().capacity() >= (count + 1) * BaseVariableWidthVector.OFFSET_WIDTH);
}
}

Expand All @@ -273,8 +272,7 @@ public void testLargeVarCharAllocateNew() throws Exception {

// verify that the validity buffer and value buffer have capacity for at least 'count' elements.
assertTrue(vector.getValidityBuffer().capacity() >= DataSizeRoundingUtil.divideBy8Ceil(count));
assertTrue(vector.getOffsetBuffer().capacity() >= (count + 1) * BaseLargeVariableWidthVector
.OFFSET_WIDTH);
assertTrue(vector.getOffsetBuffer().capacity() >= (count + 1) * BaseLargeVariableWidthVector.OFFSET_WIDTH);
}
}

Expand All @@ -287,8 +285,7 @@ public void testVarCharAllocateNewUsingHelper() throws Exception {

// verify that the validity buffer and value buffer have capacity for at least 'count' elements.
assertTrue(vector.getValidityBuffer().capacity() >= DataSizeRoundingUtil.divideBy8Ceil(count));
assertTrue(vector.getOffsetBuffer().capacity() >= (count + 1) * BaseVariableWidthVector
.OFFSET_WIDTH);
assertTrue(vector.getOffsetBuffer().capacity() >= (count + 1) * BaseVariableWidthVector.OFFSET_WIDTH);
}
}

Expand All @@ -301,8 +298,7 @@ public void testLargeVarCharAllocateNewUsingHelper() throws Exception {

// verify that the validity buffer and value buffer have capacity for at least 'count' elements.
assertTrue(vector.getValidityBuffer().capacity() >= DataSizeRoundingUtil.divideBy8Ceil(count));
assertTrue(vector.getOffsetBuffer().capacity() >= (count + 1) * BaseLargeVariableWidthVector
.OFFSET_WIDTH);
assertTrue(vector.getOffsetBuffer().capacity() >= (count + 1) * BaseLargeVariableWidthVector.OFFSET_WIDTH);
}
}

Expand Down Expand Up @@ -345,8 +341,7 @@ public void testVariableRepeatedClearAndSet() throws Exception {

@Test
public void testRepeatedValueVectorClearAndSet() throws Exception {
try (final ListVector vector = new ListVector("", allocator, FieldType.nullable(MinorType.INT.getType()),
null)) {
try (final ListVector vector = new ListVector("", allocator, FieldType.nullable(MinorType.INT.getType()), null)) {
vector.allocateNewSafe(); // Initial allocation
UnionListWriter writer = vector.getWriter();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ public void testBufferSize() {
assertEquals(overhead * count + intVector.getBufferSize() + varBinaryVector.getBufferSize(),
duv.getBufferSize());

assertEquals(overhead * (aCount + 1) + intVector.getBufferSizeFor(aCount) + varBinaryVector
.getBufferSizeFor(1),
assertEquals(overhead * (aCount + 1) + intVector.getBufferSizeFor(aCount) + varBinaryVector.getBufferSizeFor(1),
duv.getBufferSizeFor(aCount + 1));

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,34 +129,34 @@ public void testPromoteToUnion() throws Exception {
final UnionVector uv = v.getChild("A", UnionVector.class);

assertFalse(uv.isNull(0), "0 shouldn't be null");
assertEquals(uv.getObject(0), false);
assertEquals(false, uv.getObject(0));

assertFalse(uv.isNull(1), "1 shouldn't be null");
assertEquals(uv.getObject(1), true);
assertEquals(true, uv.getObject(1));

assertFalse(uv.isNull(2), "2 shouldn't be null");
assertEquals(uv.getObject(2), 10);
assertEquals(10, uv.getObject(2));

assertNull(uv.getObject(3), "3 should be null");

assertFalse(uv.isNull(4), "4 shouldn't be null");
assertEquals(uv.getObject(4), 100);
assertEquals(100, uv.getObject(4));

assertFalse(uv.isNull(5), "5 shouldn't be null");
assertEquals(uv.getObject(5), 123123L);
assertEquals(123123L, uv.getObject(5));

assertFalse(uv.isNull(6), "6 shouldn't be null");
NullableTimeStampMilliTZHolder readBackHolder = new NullableTimeStampMilliTZHolder();
uv.getTimeStampMilliTZVector().get(6, readBackHolder);
assertEquals(readBackHolder.value, 12345L);
assertEquals(readBackHolder.timezone, "UTC");
assertEquals(12345L, readBackHolder.value);
assertEquals("UTC", readBackHolder.timezone);

assertFalse(uv.isNull(7), "7 shouldn't be null");
assertEquals(((java.time.Duration) uv.getObject(7)).getSeconds(), 444413L);
assertEquals(444413L, ((java.time.Duration) uv.getObject(7)).getSeconds());

assertFalse(uv.isNull(8), "8 shouldn't be null");
assertEquals(ByteBuffer.wrap(uv.getFixedSizeBinaryVector().get(8)).order(ByteOrder.nativeOrder())
.getInt(), 18978);
assertEquals(18978,
ByteBuffer.wrap(uv.getFixedSizeBinaryVector().get(8)).order(ByteOrder.nativeOrder()).getInt());

container.clear();
container.allocateNew();
Expand Down Expand Up @@ -203,8 +203,7 @@ public void testNoPromoteFloat4ToUnionWithNull() throws Exception {
ListVector lv = ListVector.empty("name", allocator);
lv.addOrGetVector(childTypeOfListInContainer);
assertEquals(childTypeOfListInContainer.getType(), Types.MinorType.NULL.getType());
assertEquals(lv.getChildrenFromFields().get(0).getMinorType().getType(), Types.MinorType.NULL
.getType());
assertEquals(lv.getChildrenFromFields().get(0).getMinorType().getType(), Types.MinorType.NULL.getType());

writer.start();
writer.list("list").startList();
Expand All @@ -221,8 +220,7 @@ public void testNoPromoteFloat4ToUnionWithNull() throws Exception {
// we expect same behaviour from listvector
lv.addOrGetVector(childTypeOfListInContainer);
assertEquals(childTypeOfListInContainer.getType(), Types.MinorType.FLOAT4.getType());
assertEquals(lv.getChildrenFromFields().get(0).getMinorType().getType(), Types.MinorType.FLOAT4
.getType());
assertEquals(lv.getChildrenFromFields().get(0).getMinorType().getType(), Types.MinorType.FLOAT4.getType());

lv.close();
}
Expand Down Expand Up @@ -250,8 +248,7 @@ public void testNoPromoteTimeStampMilliTZToUnionWithNull() throws Exception {
ListVector lv = ListVector.empty("name", allocator);
lv.addOrGetVector(childTypeOfListInContainer);
assertEquals(childTypeOfListInContainer.getType(), Types.MinorType.NULL.getType());
assertEquals(lv.getChildrenFromFields().get(0).getMinorType().getType(), Types.MinorType.NULL
.getType());
assertEquals(lv.getChildrenFromFields().get(0).getMinorType().getType(), Types.MinorType.NULL.getType());

writer.start();
writer.list("list").startList();
Expand All @@ -264,8 +261,7 @@ public void testNoPromoteTimeStampMilliTZToUnionWithNull() throws Exception {
holder.timezone = "SomeTimeZone";
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
() -> writer.list("list").timeStampMilliTZ().write(holder));
assertEquals("holder.timezone: SomeTimeZone not equal to vector timezone: FakeTimeZone",
ex.getMessage());
assertEquals("holder.timezone: SomeTimeZone not equal to vector timezone: FakeTimeZone", ex.getMessage());

writer.list("list").endList();
writer.end();
Expand Down Expand Up @@ -309,8 +305,7 @@ public void testNoPromoteDurationToUnionWithNull() throws Exception {
ListVector lv = ListVector.empty("name", allocator);
lv.addOrGetVector(childTypeOfListInContainer);
assertEquals(childTypeOfListInContainer.getType(), Types.MinorType.NULL.getType());
assertEquals(lv.getChildrenFromFields().get(0).getMinorType().getType(), Types.MinorType.NULL
.getType());
assertEquals(lv.getChildrenFromFields().get(0).getMinorType().getType(), Types.MinorType.NULL.getType());

writer.start();
writer.list("list").startList();
Expand Down Expand Up @@ -367,8 +362,7 @@ public void testNoPromoteFixedSizeBinaryToUnionWithNull() throws Exception {
ListVector lv = ListVector.empty("name", allocator);
lv.addOrGetVector(childTypeOfListInContainer);
assertEquals(childTypeOfListInContainer.getType(), Types.MinorType.NULL.getType());
assertEquals(lv.getChildrenFromFields().get(0).getMinorType().getType(), Types.MinorType.NULL
.getType());
assertEquals(lv.getChildrenFromFields().get(0).getMinorType().getType(), Types.MinorType.NULL.getType());

writer.start();
writer.list("list").startList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ private void checkNullableStruct(NonNullableStructVector structVector) {
if (i % 2 == 0) {
assertTrue(struct.isSet(), "index is set: " + i);
assertNotNull(struct.readObject(), "index is set: " + i);
assertEquals(struct.reader("nested").readLong().longValue(), i);
assertEquals(i, struct.reader("nested").readLong().longValue());
} else {
assertFalse(struct.isSet(), "index is not set: " + i);
assertNull(struct.readObject(), "index is not set: " + i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,7 @@ protected void validateDateTimeContent(int count, VectorSchemaRoot root) {
Object timestampMilliVal = root.getVector("timestamp-milli").getObject(i);
assertEquals(dtMilli, timestampMilliVal);
Object timestampMilliTZVal = root.getVector("timestamp-milliTZ").getObject(i);
assertEquals(dt.atZone(ZoneId.of("Europe/Paris")).toInstant().toEpochMilli(),
timestampMilliTZVal);
assertEquals(dt.atZone(ZoneId.of("Europe/Paris")).toInstant().toEpochMilli(), timestampMilliTZVal);
Object timestampNanoVal = root.getVector("timestamp-nano").getObject(i);
assertEquals(dt, timestampNanoVal);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,18 +386,19 @@ public void testWriteReadWithStructDictionaries() throws IOException {
assertEquals(dictionaryVector4.getValueCount(), readDictionaryVector.getValueCount());
final BiFunction<ValueVector, ValueVector, Boolean> typeComparatorIgnoreName =
(v1, v2) -> new TypeEqualsVisitor(v1, false, true).equals(v2);
assertTrue(new RangeEqualsVisitor(dictionaryVector4, readDictionaryVector,
typeComparatorIgnoreName).rangeEquals(new Range(0, 0,
dictionaryVector4.getValueCount())),
"Dictionary vectors are not equal");
assertTrue(
new RangeEqualsVisitor(dictionaryVector4, readDictionaryVector, typeComparatorIgnoreName)
.rangeEquals(new Range(0, 0, dictionaryVector4.getValueCount())),
"Dictionary vectors are not equal");

// Assert the decoded vector is correct
try (final ValueVector readVector =
DictionaryEncoder.decode(readEncoded, readDictionary)) {
assertEquals(vector.getValueCount(), readVector.getValueCount());
assertTrue(new RangeEqualsVisitor(vector, readVector, typeComparatorIgnoreName)
assertTrue(
new RangeEqualsVisitor(vector, readVector, typeComparatorIgnoreName)
.rangeEquals(new Range(0, 0, vector.getValueCount())),
"Decoded vectors are not equal");
"Decoded vectors are not equal");
}
}
}
Expand Down
Loading

0 comments on commit d8d4af3

Please sign in to comment.