Repository: googleapis/java-spanner-jdbc
Pull request #261 (2020-11-08 10:58:16)

feat: add support for CommitStats
Adds support for `CommitStats` to the JDBC driver.

skuruppu: @olavloite I think we're almost ready to merge this. If you could please rebase and get it ready, that would be awesome.
olavloite: > @olavloite I think we're almost ready to merge this. If you could please rebase and get it ready, that would be awesome.

This should now be ready to go once Spanner client version 4.0.3 is released.
skuruppu: @olavloite Spanner Client version 5.0 is released 🎉 
codecov[bot]: # [Codecov](https://codecov.io/gh/googleapis/java-spanner-jdbc/pull/261?src=pr&el=h1) Report
> Merging [#261](https://codecov.io/gh/googleapis/java-spanner-jdbc/pull/261?src=pr&el=desc) (9a5e5c1) into [master](https://codecov.io/gh/googleapis/java-spanner-jdbc/commit/4cbee6dcc2dfea2515437b55b0ecfc956205d739?el=desc) (4cbee6d) will **increase** coverage by `0.07%`.
> The diff coverage is `55.55%`.

[![Impacted file tree graph](https://codecov.io/gh/googleapis/java-spanner-jdbc/pull/261/graphs/tree.svg?width=650&height=150&src=pr&token=ODlczvbGrj)](https://codecov.io/gh/googleapis/java-spanner-jdbc/pull/261?src=pr&el=tree)

```diff
@@             Coverage Diff              @@
##             master     #261      +/-   ##
============================================
+ Coverage     71.96%   72.03%   +0.07%     
- Complexity     1132     1135       +3     
============================================
  Files            24       24              
  Lines          3524     3533       +9     
  Branches        539      539              
============================================
+ Hits           2536     2545       +9     
  Misses          769      769              
  Partials        219      219              
```


| [Impacted Files](https://codecov.io/gh/googleapis/java-spanner-jdbc/pull/261?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../com/google/cloud/spanner/jdbc/JdbcConnection.java](https://codecov.io/gh/googleapis/java-spanner-jdbc/pull/261/diff?src=pr&el=tree#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvamRiYy9KZGJjQ29ubmVjdGlvbi5qYXZh) | `46.33% <55.55%> (+2.31%)` | `45.00 <2.00> (+3.00)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/googleapis/java-spanner-jdbc/pull/261?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/googleapis/java-spanner-jdbc/pull/261?src=pr&el=footer). Last update [4cbee6d...556bc17](https://codecov.io/gh/googleapis/java-spanner-jdbc/pull/261?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

olavloite: > @olavloite Spanner Client version 5.0 is released

@skuruppu It seems that we should not (cannot?) release this until Spanner 5.0.0 is included in the google-cloud-libraries-bom. The linkage monitor checks that the most recent versions in the bom are compatible with each other, and the current version of the bom contains Spanner 4.0.1 which is not compatible with this change.
skuruppu: > @skuruppu It seems that we should not (cannot?) release this until Spanner 5.0.0 is included in the google-cloud-libraries-bom. The linkage monitor checks that the most recent versions in the bom are compatible with each other, and the current version of the bom contains Spanner 4.0.1 which is not compatible with this change.

Ah yikes ok, we'll have to request another BOM update then. @thiagotnunes would you be able to look into this?


Patch:
diff --git a/clirr-ignored-differences.xml b/clirr-ignored-differences.xml
index 7e24377..ac9c147 100644
--- a/clirr-ignored-differences.xml
+++ b/clirr-ignored-differences.xml
@@ -94,4 +94,21 @@
     <className>com/google/cloud/spanner/jdbc/CloudSpannerJdbcConnection</className>
     <method>void setTransactionMode(com.google.cloud.spanner.connection.TransactionMode)</method>
   </difference>
+  
+  <!-- Commit stats -->
+  <difference>
+    <differenceType>7012</differenceType>
+    <className>com/google/cloud/spanner/jdbc/CloudSpannerJdbcConnection</className>
+    <method>com.google.cloud.spanner.CommitResponse getCommitResponse()</method>
+  </difference>
+  <difference>
+    <differenceType>7012</differenceType>
+    <className>com/google/cloud/spanner/jdbc/CloudSpannerJdbcConnection</className>
+    <method>boolean isReturnCommitStats()</method>
+  </difference>
+  <difference>
+    <differenceType>7012</differenceType>
+    <className>com/google/cloud/spanner/jdbc/CloudSpannerJdbcConnection</className>
+    <method>void setReturnCommitStats(boolean)</method>
+  </difference>
 </differences>
diff --git a/src/main/java/com/google/cloud/spanner/jdbc/CloudSpannerJdbcConnection.java b/src/main/java/com/google/cloud/spanner/jdbc/CloudSpannerJdbcConnection.java
index f13a64f..4a5f001 100644
--- a/src/main/java/com/google/cloud/spanner/jdbc/CloudSpannerJdbcConnection.java
+++ b/src/main/java/com/google/cloud/spanner/jdbc/CloudSpannerJdbcConnection.java
@@ -17,6 +17,8 @@
 package com.google.cloud.spanner.jdbc;
 
 import com.google.cloud.spanner.AbortedException;
+import com.google.cloud.spanner.CommitResponse;
+import com.google.cloud.spanner.CommitStats;
 import com.google.cloud.spanner.Mutation;
 import com.google.cloud.spanner.ResultSet;
 import com.google.cloud.spanner.TimestampBound;
@@ -152,6 +154,24 @@ public interface CloudSpannerJdbcConnection extends Connection {
    */
   Timestamp getCommitTimestamp() throws SQLException;
 
+  /**
+   * @return the {@link CommitResponse} of the last read/write transaction. If the last transaction
+   *     was not a read/write transaction, or a read/write transaction that did not return a {@link
+   *     CommitResponse} because the transaction was not committed, the method will throw a {@link
+   *     SQLException}. The {@link CommitResponse} will include {@link CommitStats} if {@link
+   *     #isReturnCommitStats()} returns true.
+   */
+  CommitResponse getCommitResponse() throws SQLException;
+
+  /**
+   * Sets whether this connection should request commit statistics from Cloud Spanner for read/write
+   * transactions and for DML statements in autocommit mode.
+   */
+  void setReturnCommitStats(boolean returnCommitStats) throws SQLException;
+
+  /** @return true if this connection requests commit statistics from Cloud Spanner. */
+  boolean isReturnCommitStats() throws SQLException;
+
   /**
    * @return the read {@link Timestamp} of the last read-only transaction. If the last transaction
    *     was not a read-only transaction, or a read-only transaction that did not return a read
diff --git a/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java b/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java
index 6c23db4..bf9120f 100644
--- a/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java
+++ b/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java
@@ -17,6 +17,7 @@
 package com.google.cloud.spanner.jdbc;
 
 import com.google.api.client.util.Preconditions;
+import com.google.cloud.spanner.CommitResponse;
 import com.google.cloud.spanner.Mutation;
 import com.google.cloud.spanner.SpannerException;
 import com.google.cloud.spanner.TimestampBound;
@@ -381,6 +382,36 @@ class JdbcConnection extends AbstractJdbcConnection {
     }
   }
 
+  @Override
+  public CommitResponse getCommitResponse() throws SQLException {
+    checkClosed();
+    try {
+      return getSpannerConnection().getCommitResponse();
+    } catch (SpannerException e) {
+      throw JdbcSqlExceptionFactory.of(e);
+    }
+  }
+
+  @Override
+  public void setReturnCommitStats(boolean returnCommitStats) throws SQLException {
+    checkClosed();
+    try {
+      getSpannerConnection().setReturnCommitStats(returnCommitStats);
+    } catch (SpannerException e) {
+      throw JdbcSqlExceptionFactory.of(e);
+    }
+  }
+
+  @Override
+  public boolean isReturnCommitStats() throws SQLException {
+    checkClosed();
+    try {
+      return getSpannerConnection().isReturnCommitStats();
+    } catch (SpannerException e) {
+      throw JdbcSqlExceptionFactory.of(e);
+    }
+  }
+
   @Override
   public Timestamp getReadTimestamp() throws SQLException {
     checkClosed();


Test patch:
diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcCommitStatsTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcCommitStatsTest.java
new file mode 100644
index 0000000..0782566
--- /dev/null
+++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcCommitStatsTest.java
@@ -0,0 +1,123 @@
+/*
+ * Copyright 2021 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.cloud.spanner.jdbc;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import com.google.cloud.spanner.CommitResponse;
+import com.google.cloud.spanner.Mutation;
+import com.google.cloud.spanner.connection.AbstractMockServerTest;
+import com.google.cloud.spanner.connection.SpannerPool;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import org.junit.After;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class JdbcCommitStatsTest extends AbstractMockServerTest {
+
+  @After
+  public void closeSpannerPool() {
+    SpannerPool.closeSpannerPool();
+  }
+
+  @Test
+  public void testDefaultReturnCommitStats() throws SQLException {
+    try (java.sql.Connection connection = createJdbcConnection()) {
+      try (java.sql.ResultSet rs =
+          connection.createStatement().executeQuery("SHOW VARIABLE RETURN_COMMIT_STATS")) {
+        assertTrue(rs.next());
+        assertFalse(rs.getBoolean("RETURN_COMMIT_STATS"));
+        assertFalse(rs.next());
+      }
+    }
+  }
+
+  @Test
+  public void testReturnCommitStatsInConnectionUrl() throws SQLException {
+    try (java.sql.Connection connection =
+        DriverManager.getConnection(
+            String.format("jdbc:%s;returnCommitStats=true", getBaseUrl()))) {
+      try (java.sql.ResultSet rs =
+          connection.createStatement().executeQuery("SHOW VARIABLE RETURN_COMMIT_STATS")) {
+        assertTrue(rs.next());
+        assertTrue(rs.getBoolean("RETURN_COMMIT_STATS"));
+        assertFalse(rs.next());
+      }
+    }
+  }
+
+  @Test
+  public void testSetReturnCommitStats() throws SQLException {
+    try (java.sql.Connection connection = createJdbcConnection()) {
+      connection.createStatement().execute("SET RETURN_COMMIT_STATS=true");
+      try (java.sql.ResultSet rs =
+          connection.createStatement().executeQuery("SHOW VARIABLE RETURN_COMMIT_STATS")) {
+        assertTrue(rs.next());
+        assertTrue(rs.getBoolean("RETURN_COMMIT_STATS"));
+        assertFalse(rs.next());
+      }
+      connection.createStatement().execute("SET RETURN_COMMIT_STATS=false");
+      try (java.sql.ResultSet rs =
+          connection.createStatement().executeQuery("SHOW VARIABLE RETURN_COMMIT_STATS")) {
+        assertTrue(rs.next());
+        assertFalse(rs.getBoolean("RETURN_COMMIT_STATS"));
+        assertFalse(rs.next());
+      }
+    }
+  }
+
+  @Test
+  public void testSetAndUseReturnCommitStats() throws SQLException {
+    try (CloudSpannerJdbcConnection connection =
+        createJdbcConnection().unwrap(CloudSpannerJdbcConnection.class)) {
+      connection.setReturnCommitStats(true);
+      connection.bufferedWrite(Mutation.newInsertBuilder("FOO").set("ID").to(1L).build());
+      connection.commit();
+      CommitResponse response = connection.getCommitResponse();
+      assertNotNull(response);
+      assertNotNull(response.getCommitStats());
+      assertThat(response.getCommitStats().getMutationCount()).isAtLeast(1);
+    }
+  }
+
+  @Test
+  public void testSetAndUseReturnCommitStatsUsingSql() throws SQLException {
+    try (java.sql.Connection connection = createJdbcConnection()) {
+      connection.createStatement().execute("SET RETURN_COMMIT_STATS=true");
+      // Use a Mutation as the mock server only returns a non-zero mutation count for mutations, and
+      // not for DML statements.
+      connection
+          .unwrap(CloudSpannerJdbcConnection.class)
+          .bufferedWrite(Mutation.newInsertBuilder("FOO").set("ID").to(1L).build());
+      connection.commit();
+      try (ResultSet rs =
+          connection.createStatement().executeQuery("SHOW VARIABLE COMMIT_RESPONSE")) {
+        assertTrue(rs.next());
+        assertNotNull(rs.getTimestamp("COMMIT_TIMESTAMP"));
+        assertThat(rs.getLong("MUTATION_COUNT")).isAtLeast(1L);
+        assertFalse(rs.next());
+      }
+    }
+  }
+}
diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java
index 27389b4..cd0f944 100644
--- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java
+++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java
@@ -18,7 +18,11 @@ package com.google.cloud.spanner.jdbc;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -51,6 +55,7 @@ import java.util.concurrent.Executor;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
+import org.mockito.Mockito;
 
 @RunWith(JUnit4.class)
 public class JdbcConnectionTest {
@@ -277,6 +282,14 @@ public class JdbcConnectionTest {
         "releaseSavepoint",
         new Class<?>[] {Savepoint.class},
         new Object[] {null});
+
+    testClosed(CloudSpannerJdbcConnection.class, "isReturnCommitStats");
+    testClosed(
+        CloudSpannerJdbcConnection.class,
+        "setReturnCommitStats",
+        new Class<?>[] {boolean.class},
+        new Object[] {true});
+    testClosed(CloudSpannerJdbcConnection.class, "getCommitResponse");
   }
 
   private void testClosed(Class<? extends Connection> clazz, String name)
@@ -705,4 +718,81 @@ public class JdbcConnectionTest {
       }
     }
   }
+
+  @Test
+  public void testIsReturnCommitStats() throws SQLException {
+    try (JdbcConnection connection = createConnection(mock(ConnectionOptions.class))) {
+      assertFalse(connection.isReturnCommitStats());
+      connection.setReturnCommitStats(true);
+      assertTrue(connection.isReturnCommitStats());
+    }
+  }
+
+  @Test
+  public void testIsReturnCommitStats_throwsSqlException() throws SQLException {
+    ConnectionOptions options = mock(ConnectionOptions.class);
+    com.google.cloud.spanner.connection.Connection spannerConnection =
+        mock(com.google.cloud.spanner.connection.Connection.class);
+    when(options.getConnection()).thenReturn(spannerConnection);
+    when(spannerConnection.isReturnCommitStats())
+        .thenThrow(
+            SpannerExceptionFactory.newSpannerException(
+                ErrorCode.FAILED_PRECONDITION, "test exception"));
+    try (JdbcConnection connection =
+        new JdbcConnection(
+            "jdbc:cloudspanner://localhost/projects/project/instances/instance/databases/database;credentialsUrl=url",
+            options)) {
+      connection.isReturnCommitStats();
+      fail("missing expected exception");
+    } catch (SQLException e) {
+      assertTrue(e instanceof JdbcSqlException);
+      assertEquals(Code.FAILED_PRECONDITION, ((JdbcSqlException) e).getCode());
+    }
+  }
+
+  @Test
+  public void testSetReturnCommitStats_throwsSqlException() throws SQLException {
+    ConnectionOptions options = mock(ConnectionOptions.class);
+    com.google.cloud.spanner.connection.Connection spannerConnection =
+        mock(com.google.cloud.spanner.connection.Connection.class);
+    when(options.getConnection()).thenReturn(spannerConnection);
+    Mockito.doThrow(
+            SpannerExceptionFactory.newSpannerException(
+                ErrorCode.FAILED_PRECONDITION, "test exception"))
+        .when(spannerConnection)
+        .setReturnCommitStats(any(boolean.class));
+    try (JdbcConnection connection =
+        new JdbcConnection(
+            "jdbc:cloudspanner://localhost/projects/project/instances/instance/databases/database;credentialsUrl=url",
+            options)) {
+      connection.setReturnCommitStats(true);
+      fail("missing expected exception");
+    } catch (SQLException e) {
+      assertTrue(e instanceof JdbcSqlException);
+      assertEquals(Code.FAILED_PRECONDITION, ((JdbcSqlException) e).getCode());
+    }
+  }
+
+  @Test
+  public void testGetCommitResponse_throwsSqlException() throws SQLException {
+    ConnectionOptions options = mock(ConnectionOptions.class);
+    com.google.cloud.spanner.connection.Connection spannerConnection =
+        mock(com.google.cloud.spanner.connection.Connection.class);
+    when(options.getConnection()).thenReturn(spannerConnection);
+    Mockito.doThrow(
+            SpannerExceptionFactory.newSpannerException(
+                ErrorCode.FAILED_PRECONDITION, "test exception"))
+        .when(spannerConnection)
+        .setReturnCommitStats(any(boolean.class));
+    try (JdbcConnection connection =
+        new JdbcConnection(
+            "jdbc:cloudspanner://localhost/projects/project/instances/instance/databases/database;credentialsUrl=url",
+            options)) {
+      connection.setReturnCommitStats(true);
+      fail("missing expected exception");
+    } catch (SQLException e) {
+      assertTrue(e instanceof JdbcSqlException);
+      assertEquals(Code.FAILED_PRECONDITION, ((JdbcSqlException) e).getCode());
+    }
+  }
 }