Skip to content

Commit 5c61cc3

Browse files
committed
fix(bigquery): preserve page token and paginate correctly in Arrow query when maxResults is set
1 parent 9884bc0 commit 5c61cc3

2 files changed

Lines changed: 20 additions & 16 deletions

File tree

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2620,11 +2620,6 @@ && getOptions().getOpenTelemetryTracer() != null) {
26202620
// Calculate row offset and determine if subsequent pages exist.
26212621
boolean hasMorePages = results.getPageToken() != null;
26222622
long initialRowOffset = (long) firstPageRows.size();
2623-
if (hasMorePages) {
2624-
if (content.getMaxResults() != null && initialRowOffset >= content.getMaxResults()) {
2625-
hasMorePages = false;
2626-
}
2627-
}
26282623

26292624
// Multi-page results: configure ArrowQueryPageFetcher for subsequent tabledata.list calls.
26302625
if (hasMorePages) {
@@ -2634,6 +2629,11 @@ && getOptions().getOpenTelemetryTracer() != null) {
26342629
}
26352630
JobId jobId = JobId.fromPb(results.getJobReference());
26362631
String cursor = results.getPageToken();
2632+
Map<BigQueryRpc.Option, Object> fetcherOptions = new java.util.HashMap<>(optionMap(options));
2633+
if (content.getMaxResults() != null
2634+
&& !fetcherOptions.containsKey(BigQueryRpc.Option.MAX_RESULTS)) {
2635+
fetcherOptions.put(BigQueryRpc.Option.MAX_RESULTS, content.getMaxResults());
2636+
}
26372637
NextPageFetcher<FieldValueList> pageFetcher =
26382638
new ArrowQueryPageFetcher(
26392639
jobId,
@@ -2642,8 +2642,8 @@ && getOptions().getOpenTelemetryTracer() != null) {
26422642
arrowSchemaPojo,
26432643
getOptions(),
26442644
initialRowOffset,
2645-
content.getMaxResults(),
2646-
optionMap(options));
2645+
null,
2646+
fetcherOptions);
26472647

26482648
return newTableResultBuilder(results)
26492649
.setSchema(schema)

java-bigquery/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryImplTest.java

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3354,7 +3354,8 @@ void testQueryWithArrowFormatMultiplePagesWithMaxResults()
33543354
.build();
33553355
ReadRowsResponse streamResponse =
33563356
ReadRowsResponse.newBuilder().setArrowRecordBatch(protoBatch).build();
3357-
when(mockServerStream.iterator()).thenReturn(ImmutableList.of(streamResponse).iterator());
3357+
when(mockServerStream.iterator())
3358+
.thenAnswer(invocation -> ImmutableList.of(streamResponse).iterator());
33583359

33593360
BigQueryReadClient mockReadClient =
33603361
mock(BigQueryReadClient.class, withSettings().withoutAnnotations());
@@ -3375,25 +3376,28 @@ void testQueryWithArrowFormatMultiplePagesWithMaxResults()
33753376
Page<FieldValueList> page2 = result.getNextPage();
33763377
assertNotNull(page2);
33773378
List<FieldValueList> page2Rows = ImmutableList.copyOf(page2.getValues());
3378-
// Since maxResults is 2 and initialRowOffset is 1, page2 should only contain 1 row even though
3379-
// stream returned 2 rows
3380-
assertEquals(1, page2Rows.size());
3379+
// Since maxResults configures the page size (2 rows), page2 contains the 2 rows from the stream
3380+
assertEquals(2, page2Rows.size());
33813381
assertEquals("2", page2Rows.get(0).get(0).getStringValue());
3382-
// Since totalRowsReturned == maxResults, hasNextPage must be false
3382+
assertEquals("3", page2Rows.get(1).get(0).getStringValue());
3383+
// End of stream reached (total 3 rows read across pages 1 and 2), hasNextPage must be false
33833384
assertFalse(page2.hasNextPage());
33843385
assertNull(page2.getNextPage());
33853386

3386-
// When maxResults is 1, initialRowOffset (1) already reaches maxResults, so hasNextPage is
3387-
// false immediately
3387+
// When maxResults is 1, page token is still preserved for subsequent pages
33883388
QueryJobConfiguration configMax1 =
33893389
QueryJobConfiguration.newBuilder("SELECT id FROM test")
33903390
.setQueryResultsFormat(QueryResultsFormat.ARROW)
33913391
.setMaxResults(1L)
33923392
.build();
33933393
TableResult resultMax1 = bigquery.query(configMax1);
33943394
assertNotNull(resultMax1);
3395-
assertFalse(resultMax1.hasNextPage());
3396-
assertNull(resultMax1.getNextPage());
3395+
assertTrue(resultMax1.hasNextPage());
3396+
Page<FieldValueList> page2Max1 = resultMax1.getNextPage();
3397+
assertNotNull(page2Max1);
3398+
List<FieldValueList> page2Max1Rows = ImmutableList.copyOf(page2Max1.getValues());
3399+
assertEquals(1, page2Max1Rows.size());
3400+
assertEquals("2", page2Max1Rows.get(0).get(0).getStringValue());
33973401
}
33983402

33993403
@Test

0 commit comments

Comments
 (0)