Skip to content

Commit 101d2d0

Browse files
committed
fix(bigquery): address gemini-code-assist review feedback on Arrow page fetcher and exception-safe vector creation
1 parent 67c9810 commit 101d2d0

2 files changed

Lines changed: 79 additions & 31 deletions

File tree

java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ArrowPojoUtils.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,21 @@ static List<FieldVector> createVectors(Object arrowSchemaObj, BufferAllocator al
8383
org.apache.arrow.vector.types.pojo.Schema arrowSchema =
8484
(org.apache.arrow.vector.types.pojo.Schema) arrowSchemaObj;
8585
List<FieldVector> vectors = new ArrayList<>();
86-
for (org.apache.arrow.vector.types.pojo.Field field : arrowSchema.getFields()) {
87-
vectors.add(field.createVector(allocator));
86+
try {
87+
for (org.apache.arrow.vector.types.pojo.Field field : arrowSchema.getFields()) {
88+
vectors.add(field.createVector(allocator));
89+
}
90+
return vectors;
91+
} catch (Throwable t) {
92+
for (int i = vectors.size() - 1; i >= 0; i--) {
93+
try {
94+
vectors.get(i).close();
95+
} catch (Exception e) {
96+
// ignore
97+
}
98+
}
99+
throw t;
88100
}
89-
return vectors;
90101
}
91102

92103
private static LegacySQLTypeName arrowTypeToLegacySQLTypeName(ArrowType type) {

java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java

Lines changed: 65 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,8 @@ private static class ArrowQueryPageFetcher implements NextPageFetcher<FieldValue
293293
private transient BigQueryReadClient bqReadClient;
294294
private transient ServerStream<ReadRowsResponse> stream;
295295
private transient Iterator<ReadRowsResponse> streamIterator;
296+
private transient BufferAllocator allocator;
297+
private transient VectorSchemaRoot root;
296298
private long totalRowsReturned = 0L;
297299
private boolean streamClosed = false;
298300

@@ -324,12 +326,15 @@ public Page<FieldValueList> getNextPage() {
324326

325327
try {
326328
if (bqReadClient == null) {
327-
BigQueryReadSettings settings =
328-
BigQueryReadSettings.newBuilder()
329-
.setCredentialsProvider(
330-
FixedCredentialsProvider.create(serviceOptions.getCredentials()))
331-
.build();
332-
bqReadClient = BigQueryReadClient.create(settings);
329+
BigQueryReadSettings.Builder settingsBuilder = BigQueryReadSettings.newBuilder();
330+
if (serviceOptions.getCredentials() != null) {
331+
settingsBuilder.setCredentialsProvider(
332+
FixedCredentialsProvider.create(serviceOptions.getCredentials()));
333+
}
334+
if (serviceOptions.getUniverseDomain() != null) {
335+
settingsBuilder.setUniverseDomain(serviceOptions.getUniverseDomain());
336+
}
337+
bqReadClient = BigQueryReadClient.create(settingsBuilder.build());
333338
}
334339

335340
if (streamIterator == null) {
@@ -354,28 +359,37 @@ public Page<FieldValueList> getNextPage() {
354359
arrowSchemaPojo = org.apache.arrow.vector.types.pojo.Schema.fromJSON(arrowSchemaJson);
355360
}
356361

357-
try (BufferAllocator allocator = new RootAllocator(Long.MAX_VALUE);
358-
VectorSchemaRoot root = VectorSchemaRoot.create(arrowSchemaPojo, allocator)) {
359-
VectorLoader loader = new VectorLoader(root);
360-
361-
while (rowBatch.size() < pageSize && streamIterator.hasNext()) {
362-
ReadRowsResponse response = streamIterator.next();
363-
if (response.hasArrowRecordBatch()) {
364-
com.google.cloud.bigquery.storage.v1.ArrowRecordBatch batch =
365-
response.getArrowRecordBatch();
366-
try (ArrowRecordBatch deserializedBatch =
367-
MessageSerializer.deserializeRecordBatch(
368-
new ReadChannel(
369-
new ByteArrayReadableSeekableByteChannel(
370-
batch.getSerializedRecordBatch().toByteArray())),
371-
allocator)) {
372-
loader.load(deserializedBatch);
373-
int batchRowCount = root.getRowCount();
374-
for (int i = 0; i < batchRowCount; i++) {
375-
rowBatch.add(ArrowDeserializer.arrowRootToFieldValueList(root, i, schema));
362+
if (allocator == null) {
363+
allocator = new RootAllocator(Long.MAX_VALUE);
364+
}
365+
if (root == null) {
366+
root = VectorSchemaRoot.create(arrowSchemaPojo, allocator);
367+
}
368+
369+
VectorLoader loader = new VectorLoader(root);
370+
371+
while (rowBatch.size() < pageSize
372+
&& streamIterator.hasNext()
373+
&& (totalRowsReturned + rowBatch.size() < maxResults)) {
374+
ReadRowsResponse response = streamIterator.next();
375+
if (response.hasArrowRecordBatch()) {
376+
com.google.cloud.bigquery.storage.v1.ArrowRecordBatch batch =
377+
response.getArrowRecordBatch();
378+
try (ArrowRecordBatch deserializedBatch =
379+
MessageSerializer.deserializeRecordBatch(
380+
new ReadChannel(
381+
new ByteArrayReadableSeekableByteChannel(
382+
batch.getSerializedRecordBatch().toByteArray())),
383+
allocator)) {
384+
loader.load(deserializedBatch);
385+
int batchRowCount = root.getRowCount();
386+
for (int i = 0; i < batchRowCount; i++) {
387+
if (totalRowsReturned + rowBatch.size() >= maxResults) {
388+
break;
376389
}
377-
root.clear();
390+
rowBatch.add(ArrowDeserializer.arrowRootToFieldValueList(root, i, schema));
378391
}
392+
root.clear();
379393
}
380394
}
381395
}
@@ -406,9 +420,32 @@ public Page<FieldValueList> getNextPage() {
406420
}
407421

408422
private void closeClient() {
423+
if (root != null) {
424+
try {
425+
root.close();
426+
} catch (Exception e) {
427+
// ignore
428+
} finally {
429+
root = null;
430+
}
431+
}
432+
if (allocator != null) {
433+
try {
434+
allocator.close();
435+
} catch (Exception e) {
436+
// ignore
437+
} finally {
438+
allocator = null;
439+
}
440+
}
409441
if (bqReadClient != null) {
410-
bqReadClient.close();
411-
bqReadClient = null;
442+
try {
443+
bqReadClient.close();
444+
} catch (Exception e) {
445+
// ignore
446+
} finally {
447+
bqReadClient = null;
448+
}
412449
}
413450
streamIterator = null;
414451
stream = null;

0 commit comments

Comments
 (0)