diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 429a840b21..0ef5516d27 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -177,13 +177,6 @@ public static interface SpannerEnvironment { */ @Nonnull String getOptimizerVersion(); - - /** - * The optimizer statistics package to use. Must return an empty string to indicate that no - * value has been set. - */ - @Nonnull - String getOptimizerStatisticsPackage(); } /** @@ -193,8 +186,6 @@ public static interface SpannerEnvironment { private static class SpannerEnvironmentImpl implements SpannerEnvironment { private static final SpannerEnvironmentImpl INSTANCE = new SpannerEnvironmentImpl(); private static final String SPANNER_OPTIMIZER_VERSION_ENV_VAR = "SPANNER_OPTIMIZER_VERSION"; - private static final String SPANNER_OPTIMIZER_STATISTICS_PACKAGE_ENV_VAR = - "SPANNER_OPTIMIZER_STATISTICS_PACKAGE"; private SpannerEnvironmentImpl() {} @@ -202,12 +193,6 @@ private SpannerEnvironmentImpl() {} public String getOptimizerVersion() { return MoreObjects.firstNonNull(System.getenv(SPANNER_OPTIMIZER_VERSION_ENV_VAR), ""); } - - @Override - public String getOptimizerStatisticsPackage() { - return MoreObjects.firstNonNull( - System.getenv(SPANNER_OPTIMIZER_STATISTICS_PACKAGE_ENV_VAR), ""); - } } /** Builder for {@link SpannerOptions} instances. */ diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java index cf931c99db..f665d66ada 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java @@ -736,8 +736,7 @@ public void testBackendQueryOptions() { // Just iterate over the results to execute the query. while (rs.next()) {} } - // Check that the last query was executed using a custom optimizer version and statistics - // package. + // Check that the last query was executed using a custom optimizer version. List requests = mockSpanner.getRequests(); assertThat(requests).isNotEmpty(); assertThat(requests.get(requests.size() - 1)).isInstanceOf(ExecuteSqlRequest.class); @@ -776,8 +775,7 @@ public void testBackendQueryOptionsWithAnalyzeQuery() { while (rs.next()) {} } } - // Check that the last query was executed using a custom optimizer version and statistics - // package. + // Check that the last query was executed using a custom optimizer version. List requests = mockSpanner.getRequests(); assertThat(requests).isNotEmpty(); assertThat(requests.get(requests.size() - 1)).isInstanceOf(ExecuteSqlRequest.class); @@ -818,8 +816,7 @@ public void testBackendPartitionQueryOptions() { // Just iterate over the results to execute the query. while (rs.next()) {} } - // Check that the last query was executed using a custom optimizer version and statistics - // package. + // Check that the last query was executed using a custom optimizer version. List requests = mockSpanner.getRequests(); assertThat(requests).isNotEmpty(); assertThat(requests.get(requests.size() - 1)).isInstanceOf(ExecuteSqlRequest.class); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java index 0d5b506cc2..f8b4d67c16 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java @@ -117,7 +117,7 @@ QueryOptions getEnvironmentQueryOptions() { }.build(); try (SpannerImpl implWithQueryOptions = new SpannerImpl(rpc, optionsWithQueryOptions); - SpannerImpl implWithouQueryOptions = new SpannerImpl(rpc, optionsWithoutQueryOptions)) { + SpannerImpl implWithoutQueryOptions = new SpannerImpl(rpc, optionsWithoutQueryOptions)) { // Default query options are on a per-database basis, so we should only get the custom options // for 'db' and not for 'otherDb'. @@ -125,8 +125,8 @@ QueryOptions getEnvironmentQueryOptions() { assertThat(implWithQueryOptions.getDefaultQueryOptions(otherDb)).isEqualTo(defaultOptions); // The other Spanner instance should return default options for both databases. - assertThat(implWithouQueryOptions.getDefaultQueryOptions(db)).isEqualTo(defaultOptions); - assertThat(implWithouQueryOptions.getDefaultQueryOptions(otherDb)).isEqualTo(defaultOptions); + assertThat(implWithoutQueryOptions.getDefaultQueryOptions(db)).isEqualTo(defaultOptions); + assertThat(implWithoutQueryOptions.getDefaultQueryOptions(otherDb)).isEqualTo(defaultOptions); } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 796b5588cf..8e7f946b3b 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -437,11 +437,6 @@ public void testDefaultQueryOptions() { public String getOptimizerVersion() { return ""; } - - @Override - public String getOptimizerStatisticsPackage() { - return ""; - } }); SpannerOptions options = SpannerOptions.newBuilder() @@ -461,11 +456,6 @@ public String getOptimizerStatisticsPackage() { public String getOptimizerVersion() { return "2"; } - - @Override - public String getOptimizerStatisticsPackage() { - return ""; - } }); // Create options with '1' as the default query optimizer version. This should be overridden by // the environment variable. diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITQueryOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITQueryOptionsTest.java index 1c359a6f71..89d789e6f0 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITQueryOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITQueryOptionsTest.java @@ -28,6 +28,8 @@ import com.google.cloud.spanner.Spanner; import com.google.cloud.spanner.SpannerException; import com.google.cloud.spanner.Statement; +import com.google.cloud.spanner.TransactionContext; +import com.google.cloud.spanner.TransactionRunner.TransactionCallable; import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions; import org.junit.BeforeClass; import org.junit.ClassRule; @@ -49,7 +51,9 @@ public class ITQueryOptionsTest { @BeforeClass public static void setUpDatabase() { // Empty database. - db = env.getTestHelper().createTestDatabase(); + db = + env.getTestHelper() + .createTestDatabase("CREATE TABLE TEST (ID INT64, NAME STRING(100)) PRIMARY KEY (ID)"); client = env.getTestHelper().getDatabaseClient(db); } @@ -98,6 +102,82 @@ public void executeQuery() { } } + @Test + public void executeUpdate() { + // Query optimizer version is ignored for DML statements by the backend, but setting it does not + // cause an error. + assertThat( + client + .readWriteTransaction() + .run( + new TransactionCallable() { + @Override + public Long run(TransactionContext transaction) throws Exception { + return transaction.executeUpdate( + Statement.newBuilder("INSERT INTO TEST (ID, NAME) VALUES (@id, @name)") + .bind("id") + .to(1L) + .bind("name") + .to("One") + .withQueryOptions( + QueryOptions.newBuilder().setOptimizerVersion("1").build()) + .build()); + } + })) + .isEqualTo(1L); + + // Version 'latest' should also work. + assertThat( + client + .readWriteTransaction() + .run( + new TransactionCallable() { + @Override + public Long run(TransactionContext transaction) throws Exception { + return transaction.executeUpdate( + Statement.newBuilder("INSERT INTO TEST (ID, NAME) VALUES (@id, @name)") + .bind("id") + .to(2L) + .bind("name") + .to("Two") + .withQueryOptions( + QueryOptions.newBuilder().setOptimizerVersion("latest").build()) + .build()); + } + })) + .isEqualTo(1L); + + // Version '100000' is an invalid value, but is ignored by the backend. + assertThat( + client + .readWriteTransaction() + .run( + new TransactionCallable() { + @Override + public Long run(TransactionContext transaction) throws Exception { + return transaction.executeUpdate( + Statement.newBuilder("INSERT INTO TEST (ID, NAME) VALUES (@id, @name)") + .bind("id") + .to(3L) + .bind("name") + .to("Three") + .withQueryOptions( + QueryOptions.newBuilder().setOptimizerVersion("10000").build()) + .build()); + } + })) + .isEqualTo(1L); + + // Verify that query options are ignored with Partitioned DML as well, and that all the above + // DML INSERT statements succeeded. + assertThat( + client.executePartitionedUpdate( + Statement.newBuilder("UPDATE TEST SET NAME='updated' WHERE 1=1") + .withQueryOptions(QueryOptions.newBuilder().setOptimizerVersion("1").build()) + .build())) + .isEqualTo(3L); + } + @Test public void spannerOptions() { // Version '1' should work.