fix: remove trailing semicolons in DDL (#3879)

The Connection API should automatically remove any trailing semicolons in DDL statements. Spanner accepts trailing semicolons in queries and DML statements, but not in DDL statements. Previously, the Connection API would remove all comments and trailing semicolons from DDL statements. Comments are now supported in DDL statements, and are therefore no longer removed by the Conneciton API, but that change also unintentionally kept any trailing semicolons in the SQL string that is sent to Spanner.

Fixes https://togithub.com/cloudspannerecosystem/liquibase-spanner/issues/481

Author: olavloite

google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlClient.java

@@ -35,6 +35,7 @@
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 
 /**
  * Convenience class for executing Data Definition Language statements on transactions that support
@@ -137,7 +138,21 @@ OperationFuture<Void, UpdateDatabaseDdlMetadata> executeDdl(
       dbBuilder.setProtoDescriptors(protoDescriptors);
     }
     Database db = dbBuilder.build();
-    return dbAdminClient.updateDatabaseDdl(db, statements, null);
+    return dbAdminClient.updateDatabaseDdl(
+        db,
+        statements.stream().map(DdlClient::stripTrailingSemicolon).collect(Collectors.toList()),
+        null);
+  }
+
+  static String stripTrailingSemicolon(String input) {
+    if (!input.contains(";")) {
+      return input;
+    }
+    String trimmed = input.trim();
+    if (trimmed.endsWith(";")) {
+      return trimmed.substring(0, trimmed.length() - 1);
+    }
+    return input;
   }
 
   /** Returns true if the statement is a `CREATE DATABASE ...` statement. */

google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlTest.java

@@ -364,4 +364,47 @@ public void testSetsDefaultSequenceKindAndRetriesBatch() {
         "create table bar (id2 int64 auto_increment primary key",
         ((UpdateDatabaseDdlRequest) requests.get(0)).getStatements(1));
   }
+
+  @Test
+  public void testStripTrailingSemicolon() {
+    addUpdateDdlResponse();
+    addUpdateDdlResponse();
+    addUpdateDdlResponse();
+    addUpdateDdlResponse();
+    try (Connection connection = createConnection()) {
+      connection.execute(Statement.of("drop table foo;"));
+      connection.execute(Statement.of("drop table foo  \n\t;\n\t   "));
+      connection.execute(Statement.of("drop table foo"));
+
+      connection.startBatchDdl();
+      connection.execute(Statement.of("create table foo (id1 int64 auto_increment primary key;"));
+      connection.execute(
+          Statement.of("create table foo (id1 int64 auto_increment primary key  \n\t;\n\t  "));
+      connection.execute(Statement.of("create table foo (id2 int64 auto_increment primary key"));
+      connection.runBatch();
+    }
+    assertEquals(4, mockDatabaseAdmin.getRequests().size());
+    assertEquals(
+        "drop table foo",
+        ((UpdateDatabaseDdlRequest) mockDatabaseAdmin.getRequests().get(0)).getStatements(0));
+    assertEquals(
+        "drop table foo  \n\t",
+        ((UpdateDatabaseDdlRequest) mockDatabaseAdmin.getRequests().get(1)).getStatements(0));
+    assertEquals(
+        "drop table foo",
+        ((UpdateDatabaseDdlRequest) mockDatabaseAdmin.getRequests().get(2)).getStatements(0));
+
+    assertEquals(
+        3,
+        ((UpdateDatabaseDdlRequest) mockDatabaseAdmin.getRequests().get(3)).getStatementsCount());
+    assertEquals(
+        "create table foo (id1 int64 auto_increment primary key",
+        ((UpdateDatabaseDdlRequest) mockDatabaseAdmin.getRequests().get(3)).getStatements(0));
+    assertEquals(
+        "create table foo (id1 int64 auto_increment primary key  \n\t",
+        ((UpdateDatabaseDdlRequest) mockDatabaseAdmin.getRequests().get(3)).getStatements(1));
+    assertEquals(
+        "create table foo (id2 int64 auto_increment primary key",
+        ((UpdateDatabaseDdlRequest) mockDatabaseAdmin.getRequests().get(3)).getStatements(2));
+  }
 }