Skip to content

Commit e1326c8

Browse files
fix: Move ratio calculation for whether to use read API to avoid NPE with setUseReadAPI(false) (#2509)
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [x] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/java-bigquery/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) Fixes #2508 ☕️
1 parent 909a574 commit e1326c8

File tree

3 files changed

+38
-10
lines changed

3 files changed

+38
-10
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ If you are using Maven without the BOM, add this to your dependencies:
5353
If you are using Gradle 5.x or later, add this to your dependencies:
5454

5555
```Groovy
56-
implementation platform('com.google.cloud:libraries-bom:26.13.0')
56+
implementation platform('com.google.cloud:libraries-bom:26.14.0')
5757
5858
implementation 'com.google.cloud:google-cloud-bigquery'
5959
```

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,22 +1203,18 @@ boolean isFastQuerySupported() {
12031203

12041204
@VisibleForTesting
12051205
boolean useReadAPI(Long totalRows, Long pageRows, Schema schema, Boolean hasQueryParameters) {
1206-
if ((totalRows == null || pageRows == null)
1207-
&& Boolean.TRUE.equals(
1208-
connectionSettings
1209-
.getUseReadAPI())) { // totalRows and pageRows are returned null when the job is not
1210-
// complete
1211-
return true;
1212-
}
1213-
12141206
// Read API does not yet support Interval Type or QueryParameters
12151207
if (containsIntervalType(schema) || hasQueryParameters) {
12161208
logger.log(Level.INFO, "\n Schema has IntervalType, or QueryParameters. Disabling ReadAPI");
12171209
return false;
12181210
}
12191211

1220-
long resultRatio = totalRows / pageRows;
1212+
if (totalRows == null || pageRows == null) {
1213+
return connectionSettings.getUseReadAPI();
1214+
}
1215+
12211216
if (Boolean.TRUE.equals(connectionSettings.getUseReadAPI())) {
1217+
long resultRatio = totalRows / pageRows;
12221218
return resultRatio >= connectionSettings.getTotalToPageRowCountRatio()
12231219
&& totalRows > connectionSettings.getMinResultSize();
12241220
} else {

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ public class ConnectionImplTest {
7676
Field.newBuilder("state_name", StandardSQLTypeName.STRING)
7777
.setMode(Field.Mode.NULLABLE)
7878
.build());
79+
80+
private static final Schema QUERY_SCHEMA_WITH_INTERVAL_FIELD =
81+
Schema.of(
82+
Field.newBuilder("interval", StandardSQLTypeName.INTERVAL)
83+
.setMode(Field.Mode.NULLABLE)
84+
.build());
7985
private static final TableSchema FAST_QUERY_TABLESCHEMA = QUERY_SCHEMA.toPb();
8086
private static final BigQueryResult BQ_RS_MOCK_RES =
8187
new BigQueryResultImpl(QUERY_SCHEMA, 2, null, null);
@@ -661,6 +667,32 @@ public void testGetSubsequentQueryResultsWithJob() {
661667
.getSubsequentQueryResultsWithJob(10000L, 100L, jobId, GET_QUERY_RESULTS_RESPONSE, false);
662668
}
663669

670+
@Test
671+
public void testUseReadApi() {
672+
ConnectionSettings connectionSettingsSpy = Mockito.spy(ConnectionSettings.class);
673+
doReturn(true).when(connectionSettingsSpy).getUseReadAPI();
674+
doReturn(2).when(connectionSettingsSpy).getTotalToPageRowCountRatio();
675+
doReturn(100).when(connectionSettingsSpy).getMinResultSize();
676+
677+
connection = (ConnectionImpl) bigquery.createConnection(connectionSettingsSpy);
678+
679+
// defaults to connectionSettings.getUseReadAPI() when total/page rows are null (job is still
680+
// running)
681+
assertTrue(connection.useReadAPI(null, null, QUERY_SCHEMA, false));
682+
683+
assertFalse(connection.useReadAPI(10000L, 10000L, QUERY_SCHEMA, false));
684+
assertFalse(connection.useReadAPI(50L, 10L, QUERY_SCHEMA, false));
685+
assertTrue(connection.useReadAPI(10000L, 10L, QUERY_SCHEMA, false));
686+
687+
// interval and query parameters not supported
688+
assertFalse(connection.useReadAPI(10000L, 10L, QUERY_SCHEMA_WITH_INTERVAL_FIELD, false));
689+
assertFalse(connection.useReadAPI(10000L, 10L, QUERY_SCHEMA, true));
690+
691+
doReturn(false).when(connectionSettingsSpy).getUseReadAPI();
692+
assertFalse(connection.useReadAPI(null, null, QUERY_SCHEMA, false));
693+
assertFalse(connection.useReadAPI(10000L, 10L, QUERY_SCHEMA, false));
694+
}
695+
664696
@Test
665697
public void testGetPageCacheSize() {
666698
ConnectionImpl connectionSpy = Mockito.spy(connection);

0 commit comments

Comments
 (0)