Repository: GoogleCloudPlatform/pgadapter
Pull request #541 (2022-12-09 17:18:32)

feat: support force_autocommit
Add support for the `spanner.force_autocommit` session configuration that can be used to force a connection to use autocommit for all statements. A connection that has activated this configuration option will ignore any transaction statements and execute all statements in autocommit. This option can be used in combination with the `spanner.autocommit_dml_mode` setting to force all DML statements to use Partitioned DML. Insert statements do not support PDML, and if both these two options have been set, PGAdapter will automatically execute insert statements using normal transactions to prevent errors from occurring.

go/pgadapter-force_autocommit


codecov[bot]: # [Codecov](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) Report
> Merging [#541](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (6c6c860) into [postgresql-dialect](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/commit/e385dd3a7fb2cef2d02616853c283d4869d0f32d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (e385dd3) will **increase** coverage by `0.06%`.
> The diff coverage is `93.33%`.

```diff
@@                   Coverage Diff                    @@
##             postgresql-dialect     #541      +/-   ##
========================================================
+ Coverage                 88.25%   88.32%   +0.06%     
+ Complexity                 2061     2060       -1     
========================================================
  Files                       122      122              
  Lines                      6779     6774       -5     
  Branches                    955      948       -7     
========================================================
  Hits                       5983     5983              
+ Misses                      562      560       -2     
+ Partials                    234      231       -3     
```

| Flag | Coverage Δ | |
|---|---|---|
| all_tests | `88.32% <93.33%> (+0.06%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) | Coverage Δ | |
|---|---|---|
| [...panner/pgadapter/statements/BackendConnection.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3N0YXRlbWVudHMvQmFja2VuZENvbm5lY3Rpb24uamF2YQ==) | `92.63% <92.85%> (-0.03%)` | :arrow_down: |
| [.../cloud/spanner/pgadapter/session/SessionState.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3Nlc3Npb24vU2Vzc2lvblN0YXRlLmphdmE=) | `95.58% <100.00%> (+0.02%)` | :arrow_up: |
| [...gle/cloud/spanner/pgadapter/ConnectionHandler.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL0Nvbm5lY3Rpb25IYW5kbGVyLmphdmE=) | `86.60% <0.00%> (-0.86%)` | :arrow_down: |
| [...e/cloud/spanner/pgadapter/parsers/ArrayParser.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3BhcnNlcnMvQXJyYXlQYXJzZXIuamF2YQ==) | `78.37% <0.00%> (ø)` | |
| [...e/cloud/spanner/pgadapter/parsers/JsonbParser.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3BhcnNlcnMvSnNvbmJQYXJzZXIuamF2YQ==) | `91.89% <0.00%> (ø)` | |
| [...loud/spanner/pgadapter/utils/BinaryCopyParser.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3V0aWxzL0JpbmFyeUNvcHlQYXJzZXIuamF2YQ==) | `93.39% <0.00%> (ø)` | |
| [.../cloud/spanner/pgadapter/statements/PgCatalog.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3N0YXRlbWVudHMvUGdDYXRhbG9nLmphdmE=) | `93.93% <0.00%> (+0.09%)` | :arrow_up: |
| [...oud/spanner/pgadapter/statements/SimpleParser.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3N0YXRlbWVudHMvU2ltcGxlUGFyc2VyLmphdmE=) | `92.20% <0.00%> (+0.22%)` | :arrow_up: |
| [...dapter/statements/JdbcMetadataStatementHelper.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/541/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3N0YXRlbWVudHMvSmRiY01ldGFkYXRhU3RhdGVtZW50SGVscGVyLmphdmE=) | `83.85% <0.00%> (+2.33%)` | :arrow_up: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform)


Patch:
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/session/SessionState.java b/src/main/java/com/google/cloud/spanner/pgadapter/session/SessionState.java
index 1c05bb33..74ec2701 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/session/SessionState.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/session/SessionState.java
@@ -369,6 +369,14 @@ public class SessionState {
     this.transactionSettings = null;
   }
 
+  /**
+   * Returns whether transaction statements should be ignored and all statements should be executed
+   * in autocommit mode.
+   */
+  public boolean isForceAutocommit() {
+    return getBoolSetting("spanner", "force_autocommit", false);
+  }
+
   /** Returns the current setting for replacing pg_catalog tables with common table expressions. */
   public boolean isReplacePgCatalogTables() {
     PGSetting setting = internalGet(toKey("spanner", "replace_pg_catalog_tables"), false);
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/statements/BackendConnection.java b/src/main/java/com/google/cloud/spanner/pgadapter/statements/BackendConnection.java
index aaa0fe3a..37d5ffb5 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/statements/BackendConnection.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/statements/BackendConnection.java
@@ -40,6 +40,7 @@ import com.google.cloud.spanner.Type;
 import com.google.cloud.spanner.Type.StructField;
 import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement;
 import com.google.cloud.spanner.connection.AbstractStatementParser.StatementType;
+import com.google.cloud.spanner.connection.AutocommitDmlMode;
 import com.google.cloud.spanner.connection.Connection;
 import com.google.cloud.spanner.connection.ConnectionOptionsHelper;
 import com.google.cloud.spanner.connection.ResultSetHelper;
@@ -214,6 +215,8 @@ public class BackendConnection {
             && !spannerConnection.isInTransaction()
             && (isRollback(parsedStatement) || isCommit(parsedStatement))) {
           result.set(ROLLBACK_RESULT);
+        } else if (isTransactionStatement(parsedStatement) && sessionState.isForceAutocommit()) {
+          result.set(NO_RESULT);
         } else if (isBegin(parsedStatement) && spannerConnection.isInTransaction()) {
           // Ignore the statement as it is a no-op to execute BEGIN when we are already in a
           // transaction. TODO: Return a warning.
@@ -318,6 +321,24 @@ public class BackendConnection {
         }
         return new QueryResult(resultSet);
       }
+      return executeOnSpanner(statement);
+    }
+
+    StatementResult executeOnSpanner(Statement statement) {
+      // Do not try to execute INSERT statements using Partitioned DML if we are in force_autocommit
+      // mode. We skip this because the user has (probably) set force_autocommit for compatibility
+      // reasons, so we do not want to throw an unnecessary error.
+      if (sessionState.isForceAutocommit()
+          && !spannerConnection.isInTransaction()
+          && spannerConnection.getAutocommitDmlMode() == AutocommitDmlMode.PARTITIONED_NON_ATOMIC
+          && new SimpleParser(statement.getSql()).peekKeyword("insert")) {
+        try {
+          spannerConnection.setAutocommitDmlMode(AutocommitDmlMode.TRANSACTIONAL);
+          return spannerConnection.execute(statement);
+        } finally {
+          spannerConnection.setAutocommitDmlMode(AutocommitDmlMode.PARTITIONED_NON_ATOMIC);
+        }
+      }
       return spannerConnection.execute(statement);
     }
 
@@ -831,6 +852,10 @@ public class BackendConnection {
     }
   }
 
+  private boolean isTransactionStatement(ParsedStatement parsedStatement) {
+    return isBegin(parsedStatement) || isCommit(parsedStatement) || isRollback(parsedStatement);
+  }
+
   private static final ImmutableSet<ClientSideStatementType> DDL_BATCH_STATEMENTS =
       ImmutableSet.of(
           ClientSideStatementType.START_BATCH_DDL,
diff --git a/src/main/resources/com/google/cloud/spanner/pgadapter/session/pg_settings.txt b/src/main/resources/com/google/cloud/spanner/pgadapter/session/pg_settings.txt
index 50b31ef4..d5696308 100644
--- a/src/main/resources/com/google/cloud/spanner/pgadapter/session/pg_settings.txt
+++ b/src/main/resources/com/google/cloud/spanner/pgadapter/session/pg_settings.txt
@@ -354,4 +354,5 @@ spanner.copy_max_non_atomic_commit_size	5000000	\N	COPY / The max number of byte
 spanner.copy_commit_size_multiplier_factor	2.0	\N	COPY / Factor for estimating COPY commit size	The factor that the size of the incoming payload is multiplied with to estimate whether the current commit size will exceed the maximum commit size.	\N	user	real	default	\N	\N	\N	2.0	2.0	\N	\N	f
 spanner.copy_pipe_buffer_size	65536	\N	COPY / Buffer size for incoming COPY data messages	Internal buffer size for incoming COPY data messages.	\N	internal	integer	default	\N	\N	\N	100000000	100000000	\N	\N	f
 spanner.ddl_transaction_mode	Batch	\N	PGAdapter Options	DDL transaction mode.	\N	user	enum	default	\N	\N	{Single,Batch,AutocommitImplicitTransaction,AutocommitExplicitTransaction}	Batch	Batch	\N	\N	f
+spanner.force_autocommit	off	\N	PGAdapter Options Execute all statements in autocommit mode	Ignore all transaction statements (BEGIN / COMMIT / ROLLBACK) and execute all statements in autocommit mode.	\N	user	bool	default	\N	\N	\N	off	off	\N	\N	f
 spanner.replace_pg_catalog_tables	on	\N	PGAdapter Options	Automatically replace pg_catalog table references with Common Table Expressions.	\N	user	bool	default	\N	\N	\N	on	on	\N	\N	f


Test patch:
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/AbortedMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/AbortedMockServerTest.java
index bc811aea..1dcf872a 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/AbortedMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/AbortedMockServerTest.java
@@ -1316,7 +1316,7 @@ public class AbortedMockServerTest extends AbstractMockServerTest {
           }
           count++;
         }
-        assertEquals(358, count);
+        assertEquals(359, count);
       }
     }
   }
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
index 20767cfe..3869d7b8 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
@@ -2800,7 +2800,7 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
           }
           count++;
         }
-        assertEquals(358, count);
+        assertEquals(359, count);
       }
     }
   }
@@ -3250,6 +3250,94 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
     assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class));
   }
 
+  @Test
+  public void testForceAutocommit() throws SQLException {
+    try (Connection connection = DriverManager.getConnection(createUrl())) {
+      try (java.sql.Statement statement = connection.createStatement()) {
+        // Force PGAdapter to use autocommit mode in all cases.
+        statement.execute("set spanner.force_autocommit to on");
+        // Set the connection to transactional mode. This will be ignored by PGAdapter.
+        connection.setAutoCommit(false);
+        statement.execute(SELECT1.getSql());
+        statement.execute(UPDATE_STATEMENT.getSql());
+        statement.execute(INSERT_STATEMENT.getSql());
+      }
+    }
+    assertEquals(3, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class));
+    assertEquals(2, mockSpanner.countRequestsOfType(CommitRequest.class));
+    ExecuteSqlRequest selectRequest = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0);
+    assertEquals(selectRequest.getSql(), SELECT1.getSql());
+    assertTrue(selectRequest.getTransaction().hasSingleUse());
+    assertTrue(selectRequest.getTransaction().getSingleUse().hasReadOnly());
+    ExecuteSqlRequest updateRequest = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(1);
+    assertTrue(updateRequest.getTransaction().hasBegin());
+    assertTrue(updateRequest.getTransaction().getBegin().hasReadWrite());
+    ExecuteSqlRequest insertRequest = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(2);
+    assertTrue(insertRequest.getTransaction().hasBegin());
+    assertTrue(insertRequest.getTransaction().getBegin().hasReadWrite());
+  }
+
+  @Test
+  public void testForceAutocommitWithPdml() throws SQLException {
+    try (Connection connection = DriverManager.getConnection(createUrl())) {
+      try (java.sql.Statement statement = connection.createStatement()) {
+        // Force PGAdapter to use autocommit mode in all cases.
+        statement.execute("set spanner.force_autocommit to on");
+        statement.execute("set spanner.autocommit_dml_mode to 'partitioned_non_atomic'");
+        // Set the connection to transactional mode. This will be ignored by PGAdapter.
+        connection.setAutoCommit(false);
+        statement.execute(UPDATE_STATEMENT.getSql());
+        statement.execute(INSERT_STATEMENT.getSql());
+      }
+    }
+    assertEquals(2, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class));
+    assertEquals(1, mockSpanner.countRequestsOfType(CommitRequest.class));
+    // The update statement should use a partitioned DML transaction.
+    ExecuteSqlRequest updateRequest = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0);
+    assertFalse(updateRequest.getTransaction().hasBegin());
+    assertTrue(updateRequest.getTransaction().hasId());
+    assertEquals(1, mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
+    assertEquals(
+        1,
+        mockSpanner.getRequestsOfType(BeginTransactionRequest.class).stream()
+            .filter(request -> request.getOptions().hasPartitionedDml())
+            .count());
+    // The insert statement will use a normal read/write transaction.
+    ExecuteSqlRequest insertRequest = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(1);
+    assertTrue(insertRequest.getTransaction().hasBegin());
+    assertTrue(insertRequest.getTransaction().getBegin().hasReadWrite());
+  }
+
+  @Test
+  public void testInsertPdml() throws SQLException {
+    String sql = "insert into my_table (id, value) values (1, 'one')";
+    mockSpanner.putStatementResult(
+        StatementResult.exception(
+            Statement.of(sql),
+            Status.FAILED_PRECONDITION
+                .withDescription(
+                    "insert statements are not allowed in Partitioned DML transactions")
+                .asRuntimeException()));
+
+    try (Connection connection = DriverManager.getConnection(createUrl())) {
+      try (java.sql.Statement statement = connection.createStatement()) {
+        statement.execute("set spanner.autocommit_dml_mode to 'partitioned_non_atomic'");
+        SQLException exception = assertThrows(SQLException.class, () -> statement.execute(sql));
+        assertEquals(
+            "ERROR: insert statements are not allowed in Partitioned DML transactions",
+            exception.getMessage());
+      }
+    }
+    assertEquals(1, mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
+    assertEquals(
+        1,
+        mockSpanner.getRequestsOfType(BeginTransactionRequest.class).stream()
+            .filter(request -> request.getOptions().hasPartitionedDml())
+            .count());
+    assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class));
+    assertEquals(0, mockSpanner.countRequestsOfType(CommitRequest.class));
+  }
+
   private void verifySettingIsNull(Connection connection, String setting) throws SQLException {
     try (ResultSet resultSet =
         connection.createStatement().executeQuery(String.format("show %s", setting))) {
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/session/SessionStateTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/session/SessionStateTest.java
index 29bdd306..dac991ac 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/session/SessionStateTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/session/SessionStateTest.java
@@ -160,7 +160,7 @@ public class SessionStateTest {
   public void testGetAll() {
     SessionState state = new SessionState(mock(OptionsMetadata.class));
     List<PGSetting> allSettings = state.getAll();
-    assertEquals(357, allSettings.size());
+    assertEquals(358, allSettings.size());
   }
 
   @Test
@@ -198,7 +198,7 @@ public class SessionStateTest {
     state.setLocal("spanner", "custom_local_setting", "value2");
 
     List<PGSetting> allSettings = state.getAll();
-    assertEquals(359, allSettings.size());
+    assertEquals(360, allSettings.size());
 
     PGSetting applicationName =
         allSettings.stream()