From 40110feb22986c6b5dac6885eae7f0b331aede61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Thu, 28 Apr 2022 14:46:09 +0200 Subject: [PATCH] feat: support CREATE DATABASE in Connection API (#1845) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: support CREATE DATABASE in Connection API Adds support for the CREATE DATABASE statement in the Connection API. The statement can only be used with a SingleUseTransaction (i.e. auto commit mode). It is not supported in DDL batches. The database that is created will have the same dialect as the current database that the user is connected to. Fixes #1884 * fix: add clirr ignore * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot --- .../clirr-ignored-differences.xml | 6 ++++ .../cloud/spanner/DatabaseAdminClient.java | 33 +++++++++++++++++++ .../spanner/DatabaseAdminClientImpl.java | 20 +++++++++++ .../cloud/spanner/connection/DdlBatch.java | 3 ++ .../cloud/spanner/connection/DdlClient.java | 24 ++++++++++++++ .../connection/SingleUseTransaction.java | 15 ++++++--- .../spanner/connection/DdlBatchTest.java | 12 +++++++ .../spanner/connection/DdlClientTest.java | 21 ++++++++++++ .../connection/SingleUseTransactionTest.java | 15 +++++++++ .../spanner/connection/it/ITDdlTest.java | 23 +++++++++++++ 10 files changed, 167 insertions(+), 5 deletions(-) diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index 899b44e3546..450eacde322 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -60,4 +60,10 @@ com/google/cloud/spanner/spi/v1/SpannerRpc com.google.api.gax.longrunning.OperationFuture copyBackup(com.google.cloud.spanner.BackupId, com.google.cloud.spanner.Backup) + + 7012 + com/google/cloud/spanner/DatabaseAdminClient + com.google.api.gax.longrunning.OperationFuture createDatabase(java.lang.String, java.lang.String, com.google.cloud.spanner.Dialect, java.lang.Iterable) + + diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClient.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClient.java index d3aa8b4d18f..26c48507c45 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClient.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClient.java @@ -70,6 +70,39 @@ public interface DatabaseAdminClient { OperationFuture createDatabase( String instanceId, String databaseId, Iterable statements) throws SpannerException; + /** + * Creates a new database in a Cloud Spanner instance with the given {@link Dialect}. + * + *

Example to create database. + * + *

{@code
+   * String instanceId = "my_instance_id";
+   * String createDatabaseStatement = "CREATE DATABASE \"my-database\"";
+   * Operation op = dbAdminClient
+   *     .createDatabase(
+   *         instanceId,
+   *         createDatabaseStatement,
+   *         Dialect.POSTGRESQL
+   *         Collections.emptyList());
+   * Database db = op.waitFor().getResult();
+   * }
+ * + * @param instanceId the id of the instance in which to create the database. + * @param createDatabaseStatement the CREATE DATABASE statement for the database. This statement + * must use the dialect for the new database. + * @param dialect the dialect that the new database should use. + * @param statements DDL statements to run while creating the database, for example {@code CREATE + * TABLE MyTable ( ... )}. This should not include {@code CREATE DATABASE} statement. + */ + default OperationFuture createDatabase( + String instanceId, + String createDatabaseStatement, + Dialect dialect, + Iterable statements) + throws SpannerException { + throw new UnsupportedOperationException("Unimplemented"); + } + /** * Creates a database in a Cloud Spanner instance. Any configuration options in the {@link * Database} instance will be included in the {@link CreateDatabaseRequest}. diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java index 3285dde5f5e..86b0bdbb8de 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java @@ -332,6 +332,26 @@ public OperationFuture createDatabase( final Dialect dialect = Preconditions.checkNotNull(database.getDialect()); final String createStatement = dialect.createDatabaseStatementFor(database.getId().getDatabase()); + + return createDatabase(createStatement, database, statements); + } + + @Override + public OperationFuture createDatabase( + String instanceId, + String createDatabaseStatement, + Dialect dialect, + Iterable statements) + throws SpannerException { + Database database = + newDatabaseBuilder(DatabaseId.of(projectId, instanceId, "")).setDialect(dialect).build(); + + return createDatabase(createDatabaseStatement, database, statements); + } + + private OperationFuture createDatabase( + String createStatement, Database database, Iterable statements) + throws SpannerException { OperationFuture rawOperationFuture = rpc.createDatabase( diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java index 6d34c76fde8..0a34e6b5767 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java @@ -188,6 +188,9 @@ public ApiFuture executeDdlAsync(ParsedStatement ddl) { "Only DDL statements are allowed. \"" + ddl.getSqlWithoutComments() + "\" is not a DDL-statement."); + Preconditions.checkArgument( + !DdlClient.isCreateDatabaseStatement(ddl.getSqlWithoutComments()), + "CREATE DATABASE is not supported in DDL batches."); statements.add(ddl.getSqlWithoutComments()); return ApiFutures.immediateFuture(null); } 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 f3c9cdba037..fedf60d7a91 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 @@ -17,9 +17,14 @@ package com.google.cloud.spanner.connection; import com.google.api.gax.longrunning.OperationFuture; +import com.google.cloud.spanner.Database; import com.google.cloud.spanner.DatabaseAdminClient; +import com.google.cloud.spanner.Dialect; +import com.google.cloud.spanner.ErrorCode; +import com.google.cloud.spanner.SpannerExceptionFactory; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.spanner.admin.database.v1.CreateDatabaseMetadata; import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; import java.util.Collections; import java.util.List; @@ -79,6 +84,13 @@ private DdlClient(Builder builder) { this.databaseName = builder.databaseName; } + OperationFuture executeCreateDatabase( + String createStatement, Dialect dialect) { + Preconditions.checkArgument(isCreateDatabaseStatement(createStatement)); + return dbAdminClient.createDatabase( + instanceId, createStatement, dialect, Collections.emptyList()); + } + /** Execute a single DDL statement. */ OperationFuture executeDdl(String ddl) { return executeDdl(Collections.singletonList(ddl)); @@ -86,6 +98,18 @@ OperationFuture executeDdl(String ddl) { /** Execute a list of DDL statements as one operation. */ OperationFuture executeDdl(List statements) { + if (statements.stream().anyMatch(DdlClient::isCreateDatabaseStatement)) { + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.INVALID_ARGUMENT, "CREATE DATABASE is not supported in a DDL batch"); + } return dbAdminClient.updateDatabaseDdl(instanceId, databaseName, statements, null); } + + /** Returns true if the statement is a `CREATE DATABASE ...` statement. */ + static boolean isCreateDatabaseStatement(String statement) { + String[] tokens = statement.split("\\s+", 3); + return tokens.length >= 2 + && tokens[0].equalsIgnoreCase("CREATE") + && tokens[1].equalsIgnoreCase("DATABASE"); + } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SingleUseTransaction.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SingleUseTransaction.java index f34e6f72e71..2747b07778b 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SingleUseTransaction.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SingleUseTransaction.java @@ -44,7 +44,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.spanner.admin.database.v1.DatabaseAdminGrpc; -import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; import com.google.spanner.v1.SpannerGrpc; import java.util.concurrent.Callable; @@ -270,11 +269,17 @@ public ApiFuture executeDdlAsync(final ParsedStatement ddl) { Callable callable = () -> { try { - OperationFuture operation = - ddlClient.executeDdl(ddl.getSqlWithoutComments()); - Void res = getWithStatementTimeout(operation, ddl); + OperationFuture operation; + if (DdlClient.isCreateDatabaseStatement(ddl.getSqlWithoutComments())) { + operation = + ddlClient.executeCreateDatabase( + ddl.getSqlWithoutComments(), dbClient.getDialect()); + } else { + operation = ddlClient.executeDdl(ddl.getSqlWithoutComments()); + } + getWithStatementTimeout(operation, ddl); state = UnitOfWorkState.COMMITTED; - return res; + return null; } catch (Throwable t) { state = UnitOfWorkState.COMMIT_FAILED; throw t; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlBatchTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlBatchTest.java index 23792572d79..2496adb270b 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlBatchTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlBatchTest.java @@ -22,6 +22,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import static org.mockito.Mockito.anyList; import static org.mockito.Mockito.anyString; @@ -143,6 +144,17 @@ public void testExecuteQuery() { } } + @Test + public void testExecuteCreateDatabase() { + DdlBatch batch = createSubject(); + assertThrows( + IllegalArgumentException.class, + () -> + batch.executeDdlAsync( + AbstractStatementParser.getInstance(Dialect.GOOGLE_STANDARD_SQL) + .parse(Statement.of("CREATE DATABASE foo")))); + } + @Test public void testExecuteMetadataQuery() { Statement statement = Statement.of("SELECT * FROM INFORMATION_SCHEMA.TABLES"); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlClientTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlClientTest.java index 24b4ebf0964..a490949a31b 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlClientTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DdlClientTest.java @@ -16,6 +16,8 @@ package com.google.cloud.spanner.connection; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.anyList; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.isNull; @@ -66,4 +68,23 @@ public void testExecuteDdl() throws InterruptedException, ExecutionException { subject.executeDdl(ddlList); verify(client).updateDatabaseDdl(instanceId, databaseId, ddlList, null); } + + @Test + public void testIsCreateDatabase() { + assertTrue(DdlClient.isCreateDatabaseStatement("CREATE DATABASE foo")); + assertTrue(DdlClient.isCreateDatabaseStatement("CREATE DATABASE \"foo\"")); + assertTrue(DdlClient.isCreateDatabaseStatement("CREATE DATABASE `foo`")); + assertTrue(DdlClient.isCreateDatabaseStatement("CREATE DATABASE\tfoo")); + assertTrue(DdlClient.isCreateDatabaseStatement("CREATE DATABASE\n foo")); + assertTrue(DdlClient.isCreateDatabaseStatement("CREATE DATABASE\t\n foo")); + assertTrue(DdlClient.isCreateDatabaseStatement("CREATE DATABASE")); + assertTrue(DdlClient.isCreateDatabaseStatement("CREATE\t \n DATABASE foo")); + assertTrue(DdlClient.isCreateDatabaseStatement("create\t \n DATABASE foo")); + assertTrue(DdlClient.isCreateDatabaseStatement("create database foo")); + + assertFalse(DdlClient.isCreateDatabaseStatement("CREATE VIEW foo")); + assertFalse(DdlClient.isCreateDatabaseStatement("CREATE DATABAS foo")); + assertFalse(DdlClient.isCreateDatabaseStatement("CREATE DATABASEfoo")); + assertFalse(DdlClient.isCreateDatabaseStatement("CREATE foo")); + } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SingleUseTransactionTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SingleUseTransactionTest.java index 26a3952be28..67db5e2d8b4 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SingleUseTransactionTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SingleUseTransactionTest.java @@ -34,6 +34,7 @@ import com.google.cloud.spanner.AsyncResultSet; import com.google.cloud.spanner.CommitResponse; import com.google.cloud.spanner.DatabaseClient; +import com.google.cloud.spanner.Dialect; import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.Key; import com.google.cloud.spanner.KeySet; @@ -365,6 +366,7 @@ private SingleUseTransaction createSubject( DatabaseClient dbClient = mock(DatabaseClient.class); com.google.cloud.spanner.ReadOnlyTransaction singleUse = new SimpleReadOnlyTransaction(staleness); + when(dbClient.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); when(dbClient.singleUseReadOnlyTransaction(staleness)).thenReturn(singleUse); final TransactionContext txContext = mock(TransactionContext.class); @@ -537,6 +539,19 @@ public void testExecuteDdl() { verify(ddlClient).executeDdl(sql); } + @Test + public void testExecuteCreateDatabase() { + String sql = "CREATE DATABASE FOO"; + ParsedStatement ddl = createParsedDdl(sql); + DdlClient ddlClient = createDefaultMockDdlClient(); + when(ddlClient.executeCreateDatabase(sql, Dialect.GOOGLE_STANDARD_SQL)) + .thenReturn(mock(OperationFuture.class)); + + SingleUseTransaction singleUseTransaction = createDdlSubject(ddlClient); + get(singleUseTransaction.executeDdlAsync(ddl)); + verify(ddlClient).executeCreateDatabase(sql, Dialect.GOOGLE_STANDARD_SQL); + } + @Test public void testExecuteQuery() { for (TimestampBound staleness : getTestTimestampBounds()) { diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITDdlTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITDdlTest.java index 74c072cd760..7a9c5aa9262 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITDdlTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITDdlTest.java @@ -16,7 +16,14 @@ package com.google.cloud.spanner.connection.it; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; + +import com.google.cloud.spanner.DatabaseAdminClient; +import com.google.cloud.spanner.DatabaseNotFoundException; import com.google.cloud.spanner.ParallelIntegrationTest; +import com.google.cloud.spanner.Statement; +import com.google.cloud.spanner.connection.Connection; import com.google.cloud.spanner.connection.ITAbstractSpannerTest; import com.google.cloud.spanner.connection.SqlScriptVerifier; import org.junit.Test; @@ -34,4 +41,20 @@ public void testSqlScript() throws Exception { SqlScriptVerifier verifier = new SqlScriptVerifier(new ITConnectionProvider()); verifier.verifyStatementsInFile("ITDdlTest.sql", SqlScriptVerifier.class, false); } + + @Test + public void testCreateDatabase() { + DatabaseAdminClient client = getTestEnv().getTestHelper().getClient().getDatabaseAdminClient(); + String instance = getTestEnv().getTestHelper().getInstanceId().getInstance(); + String name = getTestEnv().getTestHelper().getUniqueDatabaseId(); + + assertThrows(DatabaseNotFoundException.class, () -> client.getDatabase(instance, name)); + + try (Connection connection = createConnection()) { + connection.execute(Statement.of(String.format("CREATE DATABASE `%s`", name))); + assertNotNull(client.getDatabase(instance, name)); + } finally { + client.dropDatabase(instance, name); + } + } }