Skip to content

Commit a2f8f13

Browse files
jhrotkojhrotko
committed
Remove get(UuidHolder)
1 parent cf43493 commit a2f8f13

File tree

8 files changed

+42
-124
lines changed

8 files changed

+42
-124
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
@@ -41,8 +41,8 @@
4141
import org.apache.arrow.vector.extension.UuidType;
4242
import org.apache.arrow.vector.holders.DurationHolder;
4343
import org.apache.arrow.vector.holders.FixedSizeBinaryHolder;
44+
import org.apache.arrow.vector.holders.NullableUuidHolder;
4445
import org.apache.arrow.vector.holders.TimeStampMilliTZHolder;
45-
import org.apache.arrow.vector.holders.UuidHolder;
4646
import org.apache.arrow.vector.types.TimeUnit;
4747
import org.apache.arrow.vector.types.Types.MinorType;
4848
import org.apache.arrow.vector.types.pojo.ArrowType;
@@ -1257,7 +1257,7 @@ public void testListVectorReaderForExtensionType() throws Exception {
12571257
reader.setPosition(0);
12581258
reader.next();
12591259
FieldReader uuidReader = reader.reader();
1260-
UuidHolder holder = new UuidHolder();
1260+
NullableUuidHolder holder = new NullableUuidHolder();
12611261
uuidReader.read(holder);
12621262
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
12631263
assertEquals(u1, actualUuid);
@@ -1298,7 +1298,7 @@ public void testCopyFromForExtensionType() throws Exception {
12981298
reader.setPosition(0);
12991299
reader.next();
13001300
FieldReader uuidReader = reader.reader();
1301-
UuidHolder holder = new UuidHolder();
1301+
NullableUuidHolder holder = new NullableUuidHolder();
13021302
uuidReader.read(holder);
13031303
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
13041304
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
@@ -43,7 +43,7 @@
4343
import org.apache.arrow.vector.complex.writer.FieldWriter;
4444
import org.apache.arrow.vector.extension.UuidType;
4545
import org.apache.arrow.vector.holders.FixedSizeBinaryHolder;
46-
import org.apache.arrow.vector.holders.UuidHolder;
46+
import org.apache.arrow.vector.holders.NullableUuidHolder;
4747
import org.apache.arrow.vector.types.Types.MinorType;
4848
import org.apache.arrow.vector.types.pojo.ArrowType;
4949
import org.apache.arrow.vector.types.pojo.Field;
@@ -1302,7 +1302,7 @@ public void testMapVectorWithExtensionType() throws Exception {
13021302
mapReader.setPosition(0);
13031303
mapReader.next();
13041304
FieldReader uuidReader = mapReader.value();
1305-
UuidHolder holder = new UuidHolder();
1305+
NullableUuidHolder holder = new NullableUuidHolder();
13061306
uuidReader.read(holder);
13071307
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
13081308
assertEquals(u1, actualUuid);
@@ -1347,7 +1347,7 @@ public void testCopyFromForExtensionType() throws Exception {
13471347
mapReader.setPosition(0);
13481348
mapReader.next();
13491349
FieldReader uuidReader = mapReader.value();
1350-
UuidHolder holder = new UuidHolder();
1350+
NullableUuidHolder holder = new NullableUuidHolder();
13511351
uuidReader.read(holder);
13521352
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
13531353
assertEquals(u1, actualUuid);

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

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

255-
UuidHolder holder = new UuidHolder();
255+
NullableUuidHolder holder = new NullableUuidHolder();
256256
reader.read(holder);
257257

258258
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
@@ -310,7 +310,7 @@ void testReaderReadWithArrayIndexUuidHolder() throws Exception {
310310

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

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

316316
UUID actualUuid = UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start);
@@ -368,25 +368,6 @@ void testReaderReadWithUnsupportedHolder() throws Exception {
368368
}
369369
}
370370

371-
@Test
372-
void testReaderReadWithArrayIndexUnsupportedHolder() throws Exception {
373-
try (UuidVector vector = new UuidVector("test", allocator)) {
374-
UUID uuid = UUID.randomUUID();
375-
vector.setSafe(0, uuid);
376-
vector.setValueCount(1);
377-
378-
UuidReaderImpl reader = (UuidReaderImpl) vector.getReader();
379-
380-
// Create a mock unsupported holder
381-
ExtensionHolder unsupportedHolder = new ExtensionHolder() {};
382-
383-
IllegalArgumentException exception =
384-
assertThrows(IllegalArgumentException.class, () -> reader.read(0, unsupportedHolder));
385-
386-
assertTrue(exception.getMessage().contains("Unsupported holder type for UuidReader"));
387-
}
388-
}
389-
390371
@Test
391372
void testReaderIsSet() throws Exception {
392373
try (UuidVector vector = new UuidVector("test", allocator)) {
@@ -463,24 +444,18 @@ void testHolderStartOffsetWithMultipleValues() throws Exception {
463444
vector.setValueCount(3);
464445

465446
// Test UuidHolder with different indices
466-
UuidHolder holder1 = new UuidHolder();
467-
vector.get(0, holder1);
468-
assertEquals(0, holder1.start);
469-
assertEquals(uuid1, UuidUtility.uuidFromArrowBuf(holder1.buffer, holder1.start));
470-
471-
UuidHolder holder2 = new UuidHolder();
472-
vector.get(1, holder2);
473-
assertEquals(16, holder2.start); // UUID_BYTE_WIDTH = 16
474-
assertEquals(uuid2, UuidUtility.uuidFromArrowBuf(holder2.buffer, holder2.start));
447+
NullableUuidHolder holder = new NullableUuidHolder();
448+
vector.get(0, holder);
449+
assertEquals(0, holder.start);
450+
assertEquals(uuid1, UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start));
475451

476-
UuidHolder holder3 = new UuidHolder();
477-
vector.get(2, holder3);
478-
assertEquals(32, holder3.start); // 2 * UUID_BYTE_WIDTH = 32
479-
assertEquals(uuid3, UuidUtility.uuidFromArrowBuf(holder3.buffer, holder3.start));
452+
vector.get(1, holder);
453+
assertEquals(16, holder.start); // UUID_BYTE_WIDTH = 16
454+
assertEquals(uuid2, UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start));
480455

481-
// Verify all holders share the same buffer
482-
assertEquals(holder1.buffer, holder2.buffer);
483-
assertEquals(holder2.buffer, holder3.buffer);
456+
vector.get(2, holder);
457+
assertEquals(32, holder.start); // 2 * UUID_BYTE_WIDTH = 32
458+
assertEquals(uuid3, UuidUtility.uuidFromArrowBuf(holder.buffer, holder.start));
484459
}
485460
}
486461

@@ -504,7 +479,6 @@ void testNullableHolderStartOffsetWithMultipleValues() throws Exception {
504479

505480
NullableUuidHolder holder2 = new NullableUuidHolder();
506481
vector.get(1, holder2);
507-
assertEquals(16, holder2.start); // UUID_BYTE_WIDTH = 16
508482
assertEquals(0, holder2.isSet);
509483

510484
NullableUuidHolder holder3 = new NullableUuidHolder();
@@ -514,8 +488,7 @@ void testNullableHolderStartOffsetWithMultipleValues() throws Exception {
514488
assertEquals(uuid2, UuidUtility.uuidFromArrowBuf(holder3.buffer, holder3.start));
515489

516490
// Verify all holders share the same buffer
517-
assertEquals(holder1.buffer, holder2.buffer);
518-
assertEquals(holder2.buffer, holder3.buffer);
491+
assertEquals(holder1.buffer, holder3.buffer);
519492
}
520493
}
521494

@@ -525,15 +498,13 @@ void testSetFromHolderWithStartOffset() throws Exception {
525498
UuidVector targetVector = new UuidVector("target", allocator)) {
526499
UUID uuid1 = UUID.randomUUID();
527500
UUID uuid2 = UUID.randomUUID();
528-
UUID uuid3 = UUID.randomUUID();
529501

530502
sourceVector.setSafe(0, uuid1);
531503
sourceVector.setSafe(1, uuid2);
532-
sourceVector.setSafe(2, uuid3);
533504
sourceVector.setValueCount(3);
534505

535506
// Get holder from index 1 (should have start = 16)
536-
UuidHolder holder = new UuidHolder();
507+
NullableUuidHolder holder = new NullableUuidHolder();
537508
sourceVector.get(1, holder);
538509
assertEquals(16, holder.start);
539510

@@ -574,7 +545,6 @@ void testSetFromNullableHolderWithStartOffset() throws Exception {
574545
// Test with null holder
575546
NullableUuidHolder nullHolder = new NullableUuidHolder();
576547
sourceVector.get(1, nullHolder);
577-
assertEquals(16, nullHolder.start);
578548
assertEquals(0, nullHolder.isSet);
579549

580550
targetVector.setSafe(1, nullHolder);
@@ -610,7 +580,7 @@ void testReaderWithStartOffsetMultipleReads() throws Exception {
610580
vector.setValueCount(3);
611581

612582
UuidReaderImpl reader = (UuidReaderImpl) vector.getReader();
613-
UuidHolder holder = new UuidHolder();
583+
NullableUuidHolder holder = new NullableUuidHolder();
614584

615585
// Read from different positions and verify start offset
616586
reader.read(0, holder);
@@ -636,7 +606,7 @@ void testWriterWithExtensionHolder() throws Exception {
636606
sourceVector.setValueCount(1);
637607

638608
// Get holder from source
639-
UuidHolder holder = new UuidHolder();
609+
NullableUuidHolder holder = new NullableUuidHolder();
640610
sourceVector.get(0, holder);
641611

642612
// Write using UuidWriterImpl with ExtensionHolder

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@
8787
import org.apache.arrow.vector.holders.NullableFixedSizeBinaryHolder;
8888
import org.apache.arrow.vector.holders.NullableTimeStampMilliTZHolder;
8989
import org.apache.arrow.vector.holders.NullableTimeStampNanoTZHolder;
90+
import org.apache.arrow.vector.holders.NullableUuidHolder;
9091
import org.apache.arrow.vector.holders.TimeStampMilliTZHolder;
91-
import org.apache.arrow.vector.holders.UuidHolder;
9292
import org.apache.arrow.vector.types.TimeUnit;
9393
import org.apache.arrow.vector.types.Types.MinorType;
9494
import org.apache.arrow.vector.types.pojo.ArrowType;
@@ -2520,7 +2520,7 @@ public void extensionWriterReader() throws Exception {
25202520
{
25212521
FieldReader uuidReader = rootReader.reader("uuid1");
25222522
uuidReader.setPosition(0);
2523-
UuidHolder uuidHolder = new UuidHolder();
2523+
NullableUuidHolder uuidHolder = new NullableUuidHolder();
25242524
uuidReader.read(uuidHolder);
25252525
UUID actualUuid = UuidUtility.uuidFromArrowBuf(uuidHolder.buffer, 0);
25262526
assertEquals(u1, actualUuid);

0 commit comments

Comments
 (0)