From 4b0a5b18a082cb55f7093e1b5acf8f897376dfa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 16 May 2025 14:39:11 +0200 Subject: [PATCH] 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 https://github.com/cloudspannerecosystem/liquibase-spanner/issues/481 --- .../cloud/spanner/connection/DdlClient.java | 17 +++++++- .../cloud/spanner/connection/DdlTest.java | 43 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlClient.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlClient.java index ef7ad7e5cdb..d8dcb3c6ae3 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlClient.java +++ b/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 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. */ diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlTest.java index e71b912233d..3585421e32c 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlTest.java +++ b/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)); + } }