From 88f6910c133b9f40bc9ebb7b794dc0bbebe3383c Mon Sep 17 00:00:00 2001 From: Jonathan Swenson Date: Thu, 2 Feb 2023 23:12:13 -0800 Subject: [PATCH 1/6] fix: Move ratio calculation for whether to use read API to avoid NPE with setUseReadAPI(false) If a query takes longer than 10+ seconds, the returned job will not have results / totalRows thus totalRows will be null, However, if useReadAPI is false the moved line would throw a null pointer exception trying to unbox the (nullable -- and actually null) Long for division. --- .../src/main/java/com/google/cloud/bigquery/ConnectionImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java index eb0072905c..10d6987b6f 100644 --- a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java +++ b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java @@ -1217,8 +1217,8 @@ boolean useReadAPI(Long totalRows, Long pageRows, Schema schema, Boolean hasQuer return false; } - long resultRatio = totalRows / pageRows; if (Boolean.TRUE.equals(connectionSettings.getUseReadAPI())) { + long resultRatio = totalRows / pageRows; return resultRatio >= connectionSettings.getTotalToPageRowCountRatio() && totalRows > connectionSettings.getMinResultSize(); } else { From 0e821ad307484a1f54f70088aac8d56d39857dad Mon Sep 17 00:00:00 2001 From: Jonathan Swenson Date: Tue, 7 Feb 2023 10:15:13 -0500 Subject: [PATCH 2/6] Always exit early if either total rows or page rows is null To ensure that NPE is not thrown. Based on requested changes from @prash-mi --- .../java/com/google/cloud/bigquery/ConnectionImpl.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java index 10d6987b6f..9cf879d459 100644 --- a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java +++ b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java @@ -1203,12 +1203,8 @@ boolean isFastQuerySupported() { @VisibleForTesting boolean useReadAPI(Long totalRows, Long pageRows, Schema schema, Boolean hasQueryParameters) { - if ((totalRows == null || pageRows == null) - && Boolean.TRUE.equals( - connectionSettings - .getUseReadAPI())) { // totalRows and pageRows are returned null when the job is not - // complete - return true; + if (totalRows == null || pageRows == null) { + return false; } // Read API does not yet support Interval Type or QueryParameters From d6a16ed934270dec48d3a932f40993787303a3f2 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Mon, 13 Mar 2023 05:08:25 +0000 Subject: [PATCH 3/6] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index a1dfe4b043..a987d797bb 100644 --- a/README.md +++ b/README.md @@ -52,20 +52,20 @@ If you are using Maven without BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.5.0') +implementation platform('com.google.cloud:libraries-bom:26.10.0') implementation 'com.google.cloud:google-cloud-bigquery' ``` If you are using Gradle without BOM, add this to your dependencies: ```Groovy -implementation 'com.google.cloud:google-cloud-bigquery:2.21.0' +implementation 'com.google.cloud:google-cloud-bigquery:2.23.2' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-bigquery" % "2.21.0" +libraryDependencies += "com.google.cloud" % "google-cloud-bigquery" % "2.23.2" ``` ## Authentication From 4d67a8f9c7f449fe28edbb91ed44eb2518f2441e Mon Sep 17 00:00:00 2001 From: Obada Alabbadi Date: Tue, 25 Apr 2023 13:45:15 +0200 Subject: [PATCH 4/6] fix: use connectionSettings.getUseReadAPI() when row values are null Change useReadAPI to default to connectionSettings.getUseReadAPI() when either totalRows or pageRows are null (for long running queries). Also, move the Interval type/Queryparameters check earlier. Add unit testing. --- .../google/cloud/bigquery/ConnectionImpl.java | 8 ++--- .../cloud/bigquery/ConnectionImplTest.java | 31 +++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java index 9cf879d459..17a459312b 100644 --- a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java +++ b/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ConnectionImpl.java @@ -1203,16 +1203,16 @@ boolean isFastQuerySupported() { @VisibleForTesting boolean useReadAPI(Long totalRows, Long pageRows, Schema schema, Boolean hasQueryParameters) { - if (totalRows == null || pageRows == null) { - return false; - } - // Read API does not yet support Interval Type or QueryParameters if (containsIntervalType(schema) || hasQueryParameters) { logger.log(Level.INFO, "\n Schema has IntervalType, or QueryParameters. Disabling ReadAPI"); return false; } + if (totalRows == null || pageRows == null) { + return connectionSettings.getUseReadAPI(); + } + if (Boolean.TRUE.equals(connectionSettings.getUseReadAPI())) { long resultRatio = totalRows / pageRows; return resultRatio >= connectionSettings.getTotalToPageRowCountRatio() diff --git a/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/ConnectionImplTest.java b/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/ConnectionImplTest.java index 4b379629cf..741005d7e6 100644 --- a/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/ConnectionImplTest.java +++ b/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/ConnectionImplTest.java @@ -76,6 +76,12 @@ public class ConnectionImplTest { Field.newBuilder("state_name", StandardSQLTypeName.STRING) .setMode(Field.Mode.NULLABLE) .build()); + + private static final Schema QUERY_SCHEMA_WITH_INTERVAL_FIELD = + Schema.of( + Field.newBuilder("interval", StandardSQLTypeName.INTERVAL) + .setMode(Field.Mode.NULLABLE) + .build()); private static final TableSchema FAST_QUERY_TABLESCHEMA = QUERY_SCHEMA.toPb(); private static final BigQueryResult BQ_RS_MOCK_RES = new BigQueryResultImpl(QUERY_SCHEMA, 2, null, null); @@ -661,6 +667,31 @@ public void testGetSubsequentQueryResultsWithJob() { .getSubsequentQueryResultsWithJob(10000L, 100L, jobId, GET_QUERY_RESULTS_RESPONSE, false); } + @Test + public void testUseReadApi() { + ConnectionSettings connectionSettingsSpy = Mockito.spy(ConnectionSettings.class); + doReturn(true).when(connectionSettingsSpy).getUseReadAPI(); + doReturn(2).when(connectionSettingsSpy).getTotalToPageRowCountRatio(); + doReturn(100).when(connectionSettingsSpy).getMinResultSize(); + + connection = (ConnectionImpl) bigquery.createConnection(connectionSettingsSpy); + + // defaults to connectionSettings.getUseReadAPI() when total/page rows are null (job is still running) + assertTrue(connection.useReadAPI(null, null, QUERY_SCHEMA, false)); + + assertFalse(connection.useReadAPI(10000L, 10000L, QUERY_SCHEMA, false)); + assertFalse(connection.useReadAPI(50L, 10L, QUERY_SCHEMA, false)); + assertTrue(connection.useReadAPI(10000L, 10L, QUERY_SCHEMA, false)); + + // interval and query parameters not supported + assertFalse(connection.useReadAPI(10000L, 10L, QUERY_SCHEMA_WITH_INTERVAL_FIELD, false)); + assertFalse(connection.useReadAPI(10000L, 10L, QUERY_SCHEMA, true)); + + doReturn(false).when(connectionSettingsSpy).getUseReadAPI(); + assertFalse(connection.useReadAPI(null, null, QUERY_SCHEMA, false)); + assertFalse(connection.useReadAPI(10000L, 10L, QUERY_SCHEMA, false)); + } + @Test public void testGetPageCacheSize() { ConnectionImpl connectionSpy = Mockito.spy(connection); From 73e683d24ed55efaa9deef72619cc91121e244ec Mon Sep 17 00:00:00 2001 From: Obada Alabbadi Date: Thu, 4 May 2023 10:14:59 +0200 Subject: [PATCH 5/6] refactor: fix formatting for ConnectionImplTest.java --- .../java/com/google/cloud/bigquery/ConnectionImplTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/ConnectionImplTest.java b/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/ConnectionImplTest.java index 741005d7e6..d6348f053e 100644 --- a/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/ConnectionImplTest.java +++ b/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/ConnectionImplTest.java @@ -676,7 +676,8 @@ public void testUseReadApi() { connection = (ConnectionImpl) bigquery.createConnection(connectionSettingsSpy); - // defaults to connectionSettings.getUseReadAPI() when total/page rows are null (job is still running) + // defaults to connectionSettings.getUseReadAPI() when total/page rows are null (job is still + // running) assertTrue(connection.useReadAPI(null, null, QUERY_SCHEMA, false)); assertFalse(connection.useReadAPI(10000L, 10000L, QUERY_SCHEMA, false)); From 8dee8e8206e249d4519dde8008fc71bf01528123 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Thu, 4 May 2023 11:17:02 +0000 Subject: [PATCH 6/6] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 949a73cc42..9d3f2494c9 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ If you are using Maven without the BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.13.0') +implementation platform('com.google.cloud:libraries-bom:26.14.0') implementation 'com.google.cloud:google-cloud-bigquery' ```