Skip to content

Commit 25a573e

Browse files
jhrotkojhrotko
committed
Remove get(UuidHolder)
1 parent 091b1c6 commit 25a573e

File tree

8 files changed

+41
-130
lines changed

8 files changed

+41
-130
lines changed

‎performance/src/main/java/org/apache/arrow/vector/UuidVectorBenchmarks.java‎

Lines changed: 8 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.apache.arrow.memory.RootAllocator;
2323
import org.apache.arrow.vector.complex.impl.UuidWriterImpl;
2424
import org.apache.arrow.vector.holders.NullableUuidHolder;
25-
import org.apache.arrow.vector.holders.UuidHolder;
2625
import org.openjdk.jmh.annotations.Benchmark;
2726
import org.openjdk.jmh.annotations.BenchmarkMode;
2827
import org.openjdk.jmh.annotations.Mode;
@@ -75,26 +74,10 @@ public void tearDown() {
7574
@Benchmark
7675
@BenchmarkMode(Mode.AverageTime)
7776
@OutputTimeUnit(TimeUnit.MICROSECONDS)
78-
public void setWithUuidHolder() {
79-
UuidHolder holder = new UuidHolder();
80-
for (int i = 0; i < VECTOR_LENGTH; i++) {
81-
// Old version: get the holder populated with buffer reference, then set it back
82-
vector.get(i, holder);
83-
vector.setSafe(i, holder);
84-
}
85-
}
86-
87-
@Benchmark
88-
@BenchmarkMode(Mode.AverageTime)
89-
@OutputTimeUnit(TimeUnit.MICROSECONDS)
90-
public void setWithNullableUuidHolder() {
77+
public void setWithHolder() {
9178
NullableUuidHolder holder = new NullableUuidHolder();
9279
for (int i = 0; i < VECTOR_LENGTH; i++) {
93-
// Old version: get the holder populated with buffer reference, then set it back
94-
holder.isSet = i % 3 == 0 ? 0 : 1;
95-
if (holder.isSet == 1) {
96-
vector.get(i, holder);
97-
}
80+
vector.get(i, holder);
9881
vector.setSafe(i, holder);
9982
}
10083
}
@@ -114,26 +97,14 @@ public void setUuidDirectly() {
11497
public void setWithWriter() {
11598
UuidWriterImpl writer = new UuidWriterImpl(vector);
11699
for (int i = 0; i < VECTOR_LENGTH; i++) {
117-
if (i % 3 != 0) {
118-
writer.writeExtension(testUuids[i]);
119-
}
100+
writer.writeExtension(testUuids[i]);
120101
}
121102
}
122103

123104
@Benchmark
124105
@BenchmarkMode(Mode.AverageTime)
125106
@OutputTimeUnit(TimeUnit.MICROSECONDS)
126107
public void getWithUuidHolder() {
127-
UuidHolder holder = new UuidHolder();
128-
for (int i = 0; i < VECTOR_LENGTH; i++) {
129-
vector.get(i, holder);
130-
}
131-
}
132-
133-
@Benchmark
134-
@BenchmarkMode(Mode.AverageTime)
135-
@OutputTimeUnit(TimeUnit.MICROSECONDS)
136-
public void getWithNullableUuidHolder() {
137108
NullableUuidHolder holder = new NullableUuidHolder();
138109
for (int i = 0; i < VECTOR_LENGTH; i++) {
139110
vector.get(i, holder);
@@ -151,11 +122,11 @@ public void getUuidDirectly() {
151122

152123
public static void main(String[] args) throws RunnerException {
153124
Options opt =
154-
new OptionsBuilder()
155-
.include(UuidVectorBenchmarks.class.getSimpleName())
156-
.forks(1)
157-
.addProfiler(GCProfiler.class)
158-
.build();
125+
new OptionsBuilder()
126+
.include(UuidVectorBenchmarks.class.getSimpleName())
127+
.forks(1)
128+
.addProfiler(GCProfiler.class)
129+
.build();
159130

160131
new Runner(opt).run();
161132
}

‎vector/src/main/java/org/apache/arrow/vector/UuidVector.java‎

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.apache.arrow.vector.complex.impl.UuidReaderImpl;
3030
import org.apache.arrow.vector.complex.reader.FieldReader;
3131
import org.apache.arrow.vector.extension.UuidType;
32-
import org.apache.arrow.vector.holders.ExtensionHolder;
3332
import org.apache.arrow.vector.holders.NullableUuidHolder;
3433
import org.apache.arrow.vector.holders.UuidHolder;
3534
import org.apache.arrow.vector.types.pojo.Field;
@@ -149,23 +148,6 @@ public int isSet(int index) {
149148
return getUnderlyingVector().isSet(index);
150149
}
151150

152-
/**
153-
* Reads the UUID value at the given index into a UuidHolder.
154-
*
155-
* @param index the index to read from
156-
* @param holder the holder to populate with the UUID data
157-
*/
158-
public void get(int index, UuidHolder holder) {
159-
Preconditions.checkArgument(index >= 0, "Cannot get negative index in UUID vector.");
160-
if (NullCheckingForGet.NULL_CHECKING_ENABLED && this.isSet(index) == 0) {
161-
holder.isSet = 0;
162-
} else {
163-
holder.isSet = 1;
164-
holder.buffer = getDataBuffer();
165-
holder.start = getStartOffset(index);
166-
}
167-
}
168-
169151
/**
170152
* Reads the UUID value at the given index into a NullableUuidHolder.
171153
*
@@ -174,13 +156,13 @@ public void get(int index, UuidHolder holder) {
174156
*/
175157
public void get(int index, NullableUuidHolder holder) {
176158
Preconditions.checkArgument(index >= 0, "Cannot get negative index in UUID vector.");
177-
if (NullCheckingForGet.NULL_CHECKING_ENABLED && this.isSet(index) == 0) {
159+
if (isSet(index) == 0) {
178160
holder.isSet = 0;
179-
} else {
180-
holder.isSet = 1;
181-
holder.buffer = getDataBuffer();
182-
holder.start = getStartOffset(index);
161+
return;
183162
}
163+
holder.isSet = 1;
164+
holder.buffer = getDataBuffer();
165+
holder.start = getStartOffset(index);
184166
}
185167

186168
/**
@@ -243,8 +225,8 @@ public void set(int index, ArrowBuf source, int sourceOffset) {
243225

244226
BitVectorHelper.setBit(getUnderlyingVector().getValidityBuffer(), index);
245227
getUnderlyingVector()
246-
.getDataBuffer()
247-
.setBytes((long) index * UUID_BYTE_WIDTH, source, sourceOffset, UUID_BYTE_WIDTH);
228+
.getDataBuffer()
229+
.setBytes((long) index * UUID_BYTE_WIDTH, source, sourceOffset, UUID_BYTE_WIDTH);
248230
}
249231

250232
/**

‎vector/src/main/java/org/apache/arrow/vector/complex/impl/NullableUuidHolderReaderImpl.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,4 +121,3 @@ public Object readObject() {
121121
}
122122
}
123123
}
124-

‎vector/src/main/java/org/apache/arrow/vector/complex/impl/UuidReaderImpl.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,7 @@ public boolean isSet() {
6363

6464
@Override
6565
public void read(ExtensionHolder holder) {
66-
if (holder instanceof UuidHolder) {
67-
vector.get(idx(), (UuidHolder) holder);
68-
} else if (holder instanceof NullableUuidHolder) {
66+
if (holder instanceof NullableUuidHolder) {
6967
vector.get(idx(), (NullableUuidHolder) holder);
7068
} else {
7169
throw new IllegalArgumentException(
@@ -75,9 +73,7 @@ public void read(ExtensionHolder holder) {
7573

7674
@Override
7775
public void read(int arrayIndex, ExtensionHolder holder) {
78-
if (holder instanceof UuidHolder) {
79-
vector.get(arrayIndex, (UuidHolder) holder);
80-
} else if (holder instanceof NullableUuidHolder) {
76+
if (holder instanceof NullableUuidHolder) {
8177
vector.get(arrayIndex, (NullableUuidHolder) holder);
8278
} else {
8379
throw new IllegalArgumentException(

‎vector/src/test/java/org/apache/arrow/vector/TestListVector.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@
4040
import org.apache.arrow.vector.extension.UuidType;
4141
import org.apache.arrow.vector.holders.DurationHolder;
4242
import org.apache.arrow.vector.holders.FixedSizeBinaryHolder;
43+
import org.apache.arrow.vector.holders.NullableUuidHolder;
4344
import org.apache.arrow.vector.holders.TimeStampMilliTZHolder;
44-
import org.apache.arrow.vector.holders.UuidHolder;
4545
import org.apache.arrow.vector.types.TimeUnit;
4646
import org.apache.arrow.vector.types.Types.MinorType;
4747
import org.apache.arrow.vector.types.pojo.ArrowType;
@@ -1254,7 +1254,7 @@ public void testListVectorReaderForExtensionType() throws Exception {
12541254
reader.setPosition(0);
12551255
reader.next();
12561256
FieldReader uuidReader = reader.reader();
1257-
UuidHolder holder = new UuidHolder();
1257+
NullableUuidHolder holder = new NullableUuidHolder();
12581258
uuidReader.read(holder);
12591259
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
12601260
assertEquals(u1, actualUuid);
@@ -1294,7 +1294,7 @@ public void testCopyFromForExtensionType() throws Exception {
12941294
reader.setPosition(0);
12951295
reader.next();
12961296
FieldReader uuidReader = reader.reader();
1297-
UuidHolder holder = new UuidHolder();
1297+
NullableUuidHolder holder = new NullableUuidHolder();
12981298
uuidReader.read(holder);
12991299
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
13001300
assertEquals(u1, actualUuid);

‎vector/src/test/java/org/apache/arrow/vector/TestMapVector.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
import org.apache.arrow.vector.complex.writer.FieldWriter;
4343
import org.apache.arrow.vector.extension.UuidType;
4444
import org.apache.arrow.vector.holders.FixedSizeBinaryHolder;
45-
import org.apache.arrow.vector.holders.UuidHolder;
45+
import org.apache.arrow.vector.holders.NullableUuidHolder;
4646
import org.apache.arrow.vector.types.Types.MinorType;
4747
import org.apache.arrow.vector.types.pojo.ArrowType;
4848
import org.apache.arrow.vector.types.pojo.Field;
@@ -1299,7 +1299,7 @@ public void testMapVectorWithExtensionType() throws Exception {
12991299
mapReader.setPosition(0);
13001300
mapReader.next();
13011301
FieldReader uuidReader = mapReader.value();
1302-
UuidHolder holder = new UuidHolder();
1302+
NullableUuidHolder holder = new NullableUuidHolder();
13031303
uuidReader.read(holder);
13041304
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
13051305
assertEquals(u1, actualUuid);
@@ -1341,7 +1341,7 @@ public void testCopyFromForExtensionType() throws Exception {
13411341
mapReader.setPosition(0);
13421342
mapReader.next();
13431343
FieldReader uuidReader = mapReader.value();
1344-
UuidHolder holder = new UuidHolder();
1344+
NullableUuidHolder holder = new NullableUuidHolder();
13451345
uuidReader.read(holder);
13461346
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
13471347
assertEquals(u1, actualUuid);

‎vector/src/test/java/org/apache/arrow/vector/TestUuidVector.java‎

Lines changed: 17 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ void testReaderCopyAsValueExtensionVector() throws Exception {
236236
UuidReaderImpl reader = (UuidReaderImpl) vectorForRead.getReader();
237237
reader.copyAsValue(writer);
238238
UuidReaderImpl reader2 = (UuidReaderImpl) vector.getReader();
239-
UuidHolder holder = new UuidHolder();
239+
NullableUuidHolder holder = new NullableUuidHolder();
240240
reader2.read(0, holder);
241241
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
242242
assertEquals(uuid, actualUuid);
@@ -253,7 +253,7 @@ void testReaderReadWithUuidHolder() throws Exception {
253253
UuidReaderImpl reader = (UuidReaderImpl) vector.getReader();
254254
reader.setPosition(0);
255255

256-
UuidHolder holder = new UuidHolder();
256+
NullableUuidHolder holder = new NullableUuidHolder();
257257
reader.read(holder);
258258

259259
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
@@ -311,7 +311,7 @@ void testReaderReadWithArrayIndexUuidHolder() throws Exception {
311311

312312
UuidReaderImpl reader = (UuidReaderImpl) vector.getReader();
313313

314-
UuidHolder holder = new UuidHolder();
314+
NullableUuidHolder holder = new NullableUuidHolder();
315315
reader.read(1, holder);
316316

317317
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
@@ -375,31 +375,6 @@ public ArrowType type() {
375375
}
376376
}
377377

378-
@Test
379-
void testReaderReadWithArrayIndexUnsupportedHolder() throws Exception {
380-
try (UuidVector vector = new UuidVector("test", allocator)) {
381-
UUID uuid = UUID.randomUUID();
382-
vector.setSafe(0, uuid);
383-
vector.setValueCount(1);
384-
385-
UuidReaderImpl reader = (UuidReaderImpl) vector.getReader();
386-
387-
// Create a mock unsupported holder
388-
ExtensionHolder unsupportedHolder =
389-
new ExtensionHolder() {
390-
@Override
391-
public ArrowType type() {
392-
return null;
393-
}
394-
};
395-
396-
IllegalArgumentException exception =
397-
assertThrows(IllegalArgumentException.class, () -> reader.read(0, unsupportedHolder));
398-
399-
assertTrue(exception.getMessage().contains("Unsupported holder type for UuidReader"));
400-
}
401-
}
402-
403378
@Test
404379
void testReaderIsSet() throws Exception {
405380
try (UuidVector vector = new UuidVector("test", allocator)) {
@@ -476,24 +451,18 @@ void testHolderStartOffsetWithMultipleValues() throws Exception {
476451
vector.setValueCount(3);
477452

478453
// Test UuidHolder with different indices
479-
UuidHolder holder1 = new UuidHolder();
480-
vector.get(0, holder1);
481-
assertEquals(0, holder1.start);
482-
assertEquals(uuid1, UuidUtility.uuidFromArrowBuf(holder1.buffer, holder1.start));
483-
484-
UuidHolder holder2 = new UuidHolder();
485-
vector.get(1, holder2);
486-
assertEquals(16, holder2.start); // UUID_BYTE_WIDTH = 16
487-
assertEquals(uuid2, UuidUtility.uuidFromArrowBuf(holder2.buffer, holder2.start));
454+
NullableUuidHolder holder = new NullableUuidHolder();
455+
vector.get(0, holder);
456+
assertEquals(0, holder.start);
457+
assertEquals(uuid1, UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start));
488458

489-
UuidHolder holder3 = new UuidHolder();
490-
vector.get(2, holder3);
491-
assertEquals(32, holder3.start); // 2 * UUID_BYTE_WIDTH = 32
492-
assertEquals(uuid3, UuidUtility.uuidFromArrowBuf(holder3.buffer, holder3.start));
459+
vector.get(1, holder);
460+
assertEquals(16, holder.start); // UUID_BYTE_WIDTH = 16
461+
assertEquals(uuid2, UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start));
493462

494-
// Verify all holders share the same buffer
495-
assertEquals(holder1.buffer, holder2.buffer);
496-
assertEquals(holder2.buffer, holder3.buffer);
463+
vector.get(2, holder);
464+
assertEquals(32, holder.start); // 2 * UUID_BYTE_WIDTH = 32
465+
assertEquals(uuid3, UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start));
497466
}
498467
}
499468

@@ -517,7 +486,6 @@ void testNullableHolderStartOffsetWithMultipleValues() throws Exception {
517486

518487
NullableUuidHolder holder2 = new NullableUuidHolder();
519488
vector.get(1, holder2);
520-
assertEquals(16, holder2.start); // UUID_BYTE_WIDTH = 16
521489
assertEquals(0, holder2.isSet);
522490

523491
NullableUuidHolder holder3 = new NullableUuidHolder();
@@ -527,8 +495,7 @@ void testNullableHolderStartOffsetWithMultipleValues() throws Exception {
527495
assertEquals(uuid2, UuidUtility.uuidFromArrowBuf(holder3.buffer, holder3.start));
528496

529497
// Verify all holders share the same buffer
530-
assertEquals(holder1.buffer, holder2.buffer);
531-
assertEquals(holder2.buffer, holder3.buffer);
498+
assertEquals(holder1.buffer, holder3.buffer);
532499
}
533500
}
534501

@@ -538,15 +505,13 @@ void testSetFromHolderWithStartOffset() throws Exception {
538505
UuidVector targetVector = new UuidVector("target", allocator)) {
539506
UUID uuid1 = UUID.randomUUID();
540507
UUID uuid2 = UUID.randomUUID();
541-
UUID uuid3 = UUID.randomUUID();
542508

543509
sourceVector.setSafe(0, uuid1);
544510
sourceVector.setSafe(1, uuid2);
545-
sourceVector.setSafe(2, uuid3);
546511
sourceVector.setValueCount(3);
547512

548513
// Get holder from index 1 (should have start = 16)
549-
UuidHolder holder = new UuidHolder();
514+
NullableUuidHolder holder = new NullableUuidHolder();
550515
sourceVector.get(1, holder);
551516
assertEquals(16, holder.start);
552517

@@ -587,7 +552,6 @@ void testSetFromNullableHolderWithStartOffset() throws Exception {
587552
// Test with null holder
588553
NullableUuidHolder nullHolder = new NullableUuidHolder();
589554
sourceVector.get(1, nullHolder);
590-
assertEquals(16, nullHolder.start);
591555
assertEquals(0, nullHolder.isSet);
592556

593557
targetVector.setSafe(1, nullHolder);
@@ -623,7 +587,7 @@ void testReaderWithStartOffsetMultipleReads() throws Exception {
623587
vector.setValueCount(3);
624588

625589
UuidReaderImpl reader = (UuidReaderImpl) vector.getReader();
626-
UuidHolder holder = new UuidHolder();
590+
NullableUuidHolder holder = new NullableUuidHolder();
627591

628592
// Read from different positions and verify start offset
629593
reader.read(0, holder);
@@ -649,7 +613,7 @@ void testWriterWithExtensionHolder() throws Exception {
649613
sourceVector.setValueCount(1);
650614

651615
// Get holder from source
652-
UuidHolder holder = new UuidHolder();
616+
NullableUuidHolder holder = new NullableUuidHolder();
653617
sourceVector.get(0, holder);
654618

655619
// Write using UuidWriterImpl with ExtensionHolder

‎vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@
8888
import org.apache.arrow.vector.holders.NullableTimeStampNanoTZHolder;
8989
import org.apache.arrow.vector.holders.NullableUuidHolder;
9090
import org.apache.arrow.vector.holders.TimeStampMilliTZHolder;
91-
import org.apache.arrow.vector.holders.UuidHolder;
9291
import org.apache.arrow.vector.types.TimeUnit;
9392
import org.apache.arrow.vector.types.Types.MinorType;
9493
import org.apache.arrow.vector.types.pojo.ArrowType;
@@ -2536,7 +2535,7 @@ public void extensionWriterReader() throws Exception {
25362535
{
25372536
FieldReader uuidReader = rootReader.reader("uuid1");
25382537
uuidReader.setPosition(0);
2539-
UuidHolder uuidHolder = new UuidHolder();
2538+
NullableUuidHolder uuidHolder = new NullableUuidHolder();
25402539
uuidReader.read(uuidHolder);
25412540
UUID actualUuid = UuidUtility.uuidFromArrowBuf(uuidHolder.buffer, 0);
25422541
assertEquals(u1, actualUuid);

0 commit comments

Comments
 (0)