fix: remove trailing semicolons in DDL

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 cloudspannerecosystem/liquibase-spanner#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));
+  }
 }