From bbea0bfe93f48689f6ad6ac1963a760206399e44 Mon Sep 17 00:00:00 2001 From: Bradley White <14679271+devbww@users.noreply.github.com> Date: Tue, 22 Feb 2022 13:52:27 -0500 Subject: [PATCH] feat(spanner): convert QueryOptions to Options in main client API Add an `Options opts = {}` argument to `ExecuteQuery()`, `ProfileQuery()`, `ExecuteDml()`, `ProfileDml()`, `AnalyzeSql()`, and `ExecutePartitionedDml()`, and relegate their `QueryOptions` overloads to the "backwards compatibility" section. The new `Options` overloads also begin an `OptionsSpan`. Add `operator QueryOptions::Options()` to facilitate those compatibility interfaces, and add a round-trip test against `QueryOptions(Options const&)`. Then use that to implement `operator ClientOptions::Options()` to eliminate some duplication (and `client_options.cc`). Move the `SPANNER_OPTIMIZER_VERSION`/`SPANNER_OPTIMIZER_STATISTICS_PACKAGE` handling into `spanner:: DefaultOptions()`, which means that all of the `OverlayQueryOptions()` machinery can go away. "Overlay" priorities are now handled by the normal `Options`-merging process. Update the samples to use the preferred `Options` overloads. Part of #7690. Aside: The existing doxygen groupings do not appear to do what we want here, but let's continue with them for now, and fix them en masse later. --- google/cloud/spanner/CMakeLists.txt | 1 - google/cloud/spanner/client.cc | 131 +++--------- google/cloud/spanner/client.h | 200 ++++++++++++++---- google/cloud/spanner/client_options.cc | 49 ----- google/cloud/spanner/client_options.h | 2 +- google/cloud/spanner/client_test.cc | 107 ---------- .../spanner/google_cloud_cpp_spanner.bzl | 1 - google/cloud/spanner/internal/defaults.cc | 13 ++ google/cloud/spanner/query_options.cc | 14 ++ google/cloud/spanner/query_options.h | 12 +- google/cloud/spanner/query_options_test.cc | 17 ++ google/cloud/spanner/samples/samples.cc | 48 +++-- 12 files changed, 266 insertions(+), 329 deletions(-) diff --git a/google/cloud/spanner/CMakeLists.txt b/google/cloud/spanner/CMakeLists.txt index 265c2f9e14a76..4c5e3895d1aeb 100644 --- a/google/cloud/spanner/CMakeLists.txt +++ b/google/cloud/spanner/CMakeLists.txt @@ -83,7 +83,6 @@ add_library( bytes.h client.cc client.h - client_options.cc client_options.h commit_options.cc commit_options.h diff --git a/google/cloud/spanner/client.cc b/google/cloud/spanner/client.cc index cdfe95d28659d..bc350290e8e3d 100644 --- a/google/cloud/spanner/client.cc +++ b/google/cloud/spanner/client.cc @@ -97,65 +97,68 @@ StatusOr> Client::PartitionRead( partition_options}); } -RowStream Client::ExecuteQuery(SqlStatement statement, - QueryOptions const& opts) { +RowStream Client::ExecuteQuery(SqlStatement statement, Options opts) { + internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->ExecuteQuery({spanner_internal::MakeSingleUseTransaction( Transaction::ReadOnlyOptions()), std::move(statement), - OverlayQueryOptions(opts), + QueryOptions(internal::CurrentOptions()), {}}); } RowStream Client::ExecuteQuery( Transaction::SingleUseOptions transaction_options, SqlStatement statement, - QueryOptions const& opts) { + Options opts) { + internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->ExecuteQuery({spanner_internal::MakeSingleUseTransaction( std::move(transaction_options)), std::move(statement), - OverlayQueryOptions(opts), + QueryOptions(internal::CurrentOptions()), {}}); } RowStream Client::ExecuteQuery(Transaction transaction, SqlStatement statement, - QueryOptions const& opts) { + Options opts) { + internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->ExecuteQuery({std::move(transaction), std::move(statement), - OverlayQueryOptions(opts), + QueryOptions(internal::CurrentOptions()), {}}); } -RowStream Client::ExecuteQuery(QueryPartition const& partition, - QueryOptions const& opts) { +RowStream Client::ExecuteQuery(QueryPartition const& partition, Options opts) { + internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); auto params = spanner_internal::MakeSqlParams(partition); - params.query_options = OverlayQueryOptions(opts); + params.query_options = QueryOptions(internal::CurrentOptions()); return conn_->ExecuteQuery(std::move(params)); } -ProfileQueryResult Client::ProfileQuery(SqlStatement statement, - QueryOptions const& opts) { +ProfileQueryResult Client::ProfileQuery(SqlStatement statement, Options opts) { + internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->ProfileQuery({spanner_internal::MakeSingleUseTransaction( Transaction::ReadOnlyOptions()), std::move(statement), - OverlayQueryOptions(opts), + QueryOptions(internal::CurrentOptions()), {}}); } ProfileQueryResult Client::ProfileQuery( Transaction::SingleUseOptions transaction_options, SqlStatement statement, - QueryOptions const& opts) { + Options opts) { + internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->ProfileQuery({spanner_internal::MakeSingleUseTransaction( std::move(transaction_options)), std::move(statement), - OverlayQueryOptions(opts), + QueryOptions(internal::CurrentOptions()), {}}); } ProfileQueryResult Client::ProfileQuery(Transaction transaction, - SqlStatement statement, - QueryOptions const& opts) { + SqlStatement statement, Options opts) { + internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->ProfileQuery({std::move(transaction), std::move(statement), - OverlayQueryOptions(opts), + QueryOptions(internal::CurrentOptions()), {}}); } @@ -167,29 +170,31 @@ StatusOr> Client::PartitionQuery( } StatusOr Client::ExecuteDml(Transaction transaction, - SqlStatement statement, - QueryOptions const& opts) { + SqlStatement statement, Options opts) { + internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->ExecuteDml({std::move(transaction), std::move(statement), - OverlayQueryOptions(opts), + QueryOptions(internal::CurrentOptions()), {}}); } StatusOr Client::ProfileDml(Transaction transaction, SqlStatement statement, - QueryOptions const& opts) { + Options opts) { + internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->ProfileDml({std::move(transaction), std::move(statement), - OverlayQueryOptions(opts), + QueryOptions(internal::CurrentOptions()), {}}); } StatusOr Client::AnalyzeSql(Transaction transaction, SqlStatement statement, - QueryOptions const& opts) { + Options opts) { + internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->AnalyzeSql({std::move(transaction), std::move(statement), - OverlayQueryOptions(opts), + QueryOptions(internal::CurrentOptions()), {}}); } @@ -316,28 +321,10 @@ Status Client::Rollback(Transaction transaction, Options opts) { } StatusOr Client::ExecutePartitionedDml( - SqlStatement statement, QueryOptions const& opts) { + SqlStatement statement, Options opts) { + internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->ExecutePartitionedDml( - {std::move(statement), OverlayQueryOptions(opts)}); -} - -// Returns a QueryOptions that has each attribute set according to -// the hierarchy that options specified at the function call (i.e., -// `preferred`) are preferred, followed by options set at the Client -// level (i.e., `opts_.query_options()`), followed by some environment -// variables. If none are set, the attribute's optional will be unset -// and nothing will be included in the proto sent to Spanner, in which -// case the Database default will be used. -QueryOptions Client::OverlayQueryOptions(QueryOptions const& preferred) { - // GetEnv() is not super fast, so we look it up once and cache it. - static auto const* const kOptimizerVersionEnvValue = - new auto(google::cloud::internal::GetEnv("SPANNER_OPTIMIZER_VERSION")); - static auto const* const kOptimizerStatisticsPackageEnvValue = new auto( - google::cloud::internal::GetEnv("SPANNER_OPTIMIZER_STATISTICS_PACKAGE")); - - return spanner_internal::OverlayQueryOptions( - preferred, query_opts_, *kOptimizerVersionEnvValue, - *kOptimizerStatisticsPackageEnvValue); + {std::move(statement), QueryOptions(internal::CurrentOptions())}); } std::shared_ptr MakeConnection(spanner::Database const& db, @@ -384,57 +371,5 @@ std::shared_ptr MakeConnection( GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace spanner - -namespace spanner_internal { -GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN - -// Returns a QueryOptions that has each attribute set according to the -// preferred/fallback/environment hierarchy. -spanner::QueryOptions OverlayQueryOptions( - spanner::QueryOptions const& preferred, - spanner::QueryOptions const& fallback, - absl::optional const& optimizer_version_env, - absl::optional const& optimizer_statistics_package_env) { - spanner::QueryOptions opts; - - // Choose the `optimizer_version` option. - if (preferred.optimizer_version().has_value()) { - opts.set_optimizer_version(preferred.optimizer_version()); - } else if (fallback.optimizer_version().has_value()) { - opts.set_optimizer_version(fallback.optimizer_version()); - } else if (optimizer_version_env.has_value()) { - opts.set_optimizer_version(*optimizer_version_env); - } - - // Choose the `optimizer_statistics_package` option. - if (preferred.optimizer_statistics_package().has_value()) { - opts.set_optimizer_statistics_package( - preferred.optimizer_statistics_package()); - } else if (fallback.optimizer_statistics_package().has_value()) { - opts.set_optimizer_statistics_package( - fallback.optimizer_statistics_package()); - } else if (optimizer_statistics_package_env.has_value()) { - opts.set_optimizer_statistics_package(*optimizer_statistics_package_env); - } - - // Choose the `request_priority` option. - if (preferred.request_priority().has_value()) { - opts.set_request_priority(preferred.request_priority()); - } else if (fallback.request_priority().has_value()) { - opts.set_request_priority(fallback.request_priority()); - } - - // Choose the `request_tag` option. - if (preferred.request_tag().has_value()) { - opts.set_request_tag(preferred.request_tag()); - } else if (fallback.request_tag().has_value()) { - opts.set_request_tag(fallback.request_tag()); - } - - return opts; -} - -GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END -} // namespace spanner_internal } // namespace cloud } // namespace google diff --git a/google/cloud/spanner/client.h b/google/cloud/spanner/client.h index c92674c8f995b..cf1c99c8dd087 100644 --- a/google/cloud/spanner/client.h +++ b/google/cloud/spanner/client.h @@ -38,7 +38,6 @@ #include "google/cloud/spanner/version.h" #include "google/cloud/backoff_policy.h" #include "google/cloud/internal/non_constructible.h" -#include "google/cloud/optional.h" #include "google/cloud/options.h" #include "google/cloud/status.h" #include "google/cloud/status_or.h" @@ -102,18 +101,18 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN * @par Query Options * * Most operations that take an `SqlStatement` may also be modified with - * `QueryOptions`. These options can be set at various levels, with more + * query `Options`. These options can be set at various levels, with more * specific levels taking precedence over broader ones. For example, - * `QueryOptions` that are passed directly to `Client::ExecuteQuery()` will + * `Options` that are passed directly to `Client::ExecuteQuery()` will * take precedence over the `Client`-level defaults (if any), which will * themselves take precedence over any environment variables. The following * table shows the environment variables that may optionally be set and the - * `QueryOptions` setting that they affect. + * query `Options` setting that they affect. * - * Environment Variable | QueryOptions setting + * Environment Variable | Options setting * -------------------------------------- | -------------------- - * `SPANNER_OPTIMIZER_VERSION` | `QueryOptions::optimizer_version()` - * `SPANNER_OPTIMIZER_STATISTICS_PACKAGE` | `QueryOptions::optimizer_statistics_package()` + * `SPANNER_OPTIMIZER_VERSION` | `QueryOptimizerVersionOption` + * `SPANNER_OPTIMIZER_STATISTICS_PACKAGE` | `QueryOptimizerStatisticsPackageOption` * * @see https://cloud.google.com/spanner/docs/reference/rest/v1/QueryOptions * @see http://cloud/spanner/docs/query-optimizer/manage-query-optimizer @@ -294,31 +293,31 @@ class Client { * @snippet samples.cc spanner-query-data-select-star * * @param statement The SQL statement to execute. - * @param opts (optional) The `QueryOptions` to use for this call. If given, + * @param opts (optional) The `Options` to use for this call. If given, * these will take precedence over the options set at the client and * environment levels. * * @note No individual row in the `RowStream` can exceed 100 MiB, and no * column value can exceed 10 MiB. */ - RowStream ExecuteQuery(SqlStatement statement, QueryOptions const& opts = {}); + RowStream ExecuteQuery(SqlStatement statement, Options opts = {}); /** - * @copydoc ExecuteQuery(SqlStatement, QueryOptions const&) + * @copydoc ExecuteQuery(SqlStatement, Options) * * @param transaction_options Execute this query in a single-use transaction * with these options. */ RowStream ExecuteQuery(Transaction::SingleUseOptions transaction_options, - SqlStatement statement, QueryOptions const& opts = {}); + SqlStatement statement, Options opts = {}); /** - * @copydoc ExecuteQuery(SqlStatement, QueryOptions const&) + * @copydoc ExecuteQuery(SqlStatement, Options) * * @param transaction Execute this query as part of an existing transaction. */ RowStream ExecuteQuery(Transaction transaction, SqlStatement statement, - QueryOptions const& opts = {}); + Options opts = {}); /** * Executes a SQL query on a subset of rows in a database. Requires a prior @@ -326,7 +325,7 @@ class Client { * documentation of that method for full details. * * @param partition A `QueryPartition`, obtained by calling `PartitionQuery`. - * @param opts (optional) The `QueryOptions` to use for this call. If given, + * @param opts (optional) The `Options` to use for this call. If given, * these will take precedence over the options set at the client and * environment levels. * @@ -336,8 +335,45 @@ class Client { * @par Example * @snippet samples.cc execute-sql-query-partition */ + RowStream ExecuteQuery(QueryPartition const& partition, Options opts = {}); + //@} + + //@{ + /// Backwards compatibility for `QueryOptions`. + RowStream ExecuteQuery(SqlStatement statement, QueryOptions const& opts) { + return ExecuteQuery(std::move(statement), Options(opts)); + } + RowStream ExecuteQuery(SqlStatement statement, + std::initializer_list) { + return ExecuteQuery(std::move(statement)); + } + RowStream ExecuteQuery(Transaction::SingleUseOptions transaction_options, + SqlStatement statement, QueryOptions const& opts) { + return ExecuteQuery(std::move(transaction_options), std::move(statement), + Options(opts)); + } + RowStream ExecuteQuery(Transaction::SingleUseOptions transaction_options, + SqlStatement statement, + std::initializer_list) { + return ExecuteQuery(std::move(transaction_options), std::move(statement)); + } + RowStream ExecuteQuery(Transaction transaction, SqlStatement statement, + QueryOptions const& opts) { + return ExecuteQuery(std::move(transaction), std::move(statement), + Options(opts)); + } + RowStream ExecuteQuery(Transaction transaction, SqlStatement statement, + std::initializer_list) { + return ExecuteQuery(std::move(transaction), std::move(statement)); + } + RowStream ExecuteQuery(QueryPartition const& partition, + QueryOptions const& opts) { + return ExecuteQuery(partition, Options(opts)); + } RowStream ExecuteQuery(QueryPartition const& partition, - QueryOptions const& opts = {}); + std::initializer_list) { + return ExecuteQuery(partition); + } //@} //@{ @@ -359,7 +395,7 @@ class Client { * statistics and `ExecutionPlan` are available. * * @param statement The SQL statement to execute. - * @param opts (optional) The `QueryOptions` to use for this call. If given, + * @param opts (optional) The `Options` to use for this call. If given, * these will take precedence over the options set at the client and * environment levels. * @@ -369,27 +405,60 @@ class Client { * @par Example * @snippet samples.cc profile-query */ - ProfileQueryResult ProfileQuery(SqlStatement statement, - QueryOptions const& opts = {}); + ProfileQueryResult ProfileQuery(SqlStatement statement, Options opts = {}); /** - * @copydoc ProfileQuery(SqlStatement, QueryOptions const&) + * @copydoc ProfileQuery(SqlStatement, Options) * * @param transaction_options Execute this query in a single-use transaction * with these options. */ ProfileQueryResult ProfileQuery( Transaction::SingleUseOptions transaction_options, SqlStatement statement, - QueryOptions const& opts = {}); + Options opts = {}); /** - * @copydoc ProfileQuery(SqlStatement, QueryOptions const&) + * @copydoc ProfileQuery(SqlStatement, Options) * * @param transaction Execute this query as part of an existing transaction. */ + ProfileQueryResult ProfileQuery(Transaction transaction, + SqlStatement statement, Options opts = {}); + //@} + + //@{ + /// Backwards compatibility for `QueryOptions`. + ProfileQueryResult ProfileQuery(SqlStatement statement, + QueryOptions const& opts) { + return ProfileQuery(std::move(statement), Options(opts)); + } + ProfileQueryResult ProfileQuery( + SqlStatement statement, + std::initializer_list) { + return ProfileQuery(std::move(statement)); + } + ProfileQueryResult ProfileQuery( + Transaction::SingleUseOptions transaction_options, SqlStatement statement, + QueryOptions const& opts) { + return ProfileQuery(std::move(transaction_options), std::move(statement), + Options(opts)); + } + ProfileQueryResult ProfileQuery( + Transaction::SingleUseOptions transaction_options, SqlStatement statement, + std::initializer_list) { + return ProfileQuery(std::move(transaction_options), std::move(statement)); + } ProfileQueryResult ProfileQuery(Transaction transaction, SqlStatement statement, - QueryOptions const& opts = {}); + QueryOptions const& opts) { + return ProfileQuery(std::move(transaction), std::move(statement), + Options(opts)); + } + ProfileQueryResult ProfileQuery( + Transaction transaction, SqlStatement statement, + std::initializer_list) { + return ProfileQuery(std::move(transaction), std::move(statement)); + } //@} /** @@ -427,16 +496,30 @@ class Client { * * @param transaction Execute this query as part of an existing transaction. * @param statement The SQL statement to execute. - * @param opts (optional) The `QueryOptions` to use for this call. If given, + * @param opts (optional) The `Options` to use for this call. If given, * these will take precedence over the options set at the client and * environment levels. * * @par Example * @snippet samples.cc execute-dml */ + StatusOr ExecuteDml(Transaction transaction, + SqlStatement statement, Options opts = {}); + + //@{ + /// Backwards compatibility for `QueryOptions`. StatusOr ExecuteDml(Transaction transaction, SqlStatement statement, - QueryOptions const& opts = {}); + QueryOptions const& opts) { + return ExecuteDml(std::move(transaction), std::move(statement), + Options(opts)); + } + StatusOr ExecuteDml( + Transaction transaction, SqlStatement statement, + std::initializer_list) { + return ExecuteDml(std::move(transaction), std::move(statement)); + } + //@} /** * Profiles a SQL DML statement. @@ -451,7 +534,7 @@ class Client { * * @param transaction Execute this query as part of an existing transaction. * @param statement The SQL statement to execute. - * @param opts (optional) The `QueryOptions` to use for this call. If given, + * @param opts (optional) The `Options` to use for this call. If given, * these will take precedence over the options set at the client and * environment levels. * @@ -460,7 +543,22 @@ class Client { */ StatusOr ProfileDml(Transaction transaction, SqlStatement statement, - QueryOptions const& opts = {}); + Options opts = {}); + + //@{ + /// Backwards compatibility for `QueryOptions`. + StatusOr ProfileDml(Transaction transaction, + SqlStatement statement, + QueryOptions const& opts) { + return ProfileDml(std::move(transaction), std::move(statement), + Options(opts)); + } + StatusOr ProfileDml( + Transaction transaction, SqlStatement statement, + std::initializer_list) { + return ProfileDml(std::move(transaction), std::move(statement)); + } + //@} /** * Analyzes the execution plan of a SQL statement. @@ -475,16 +573,30 @@ class Client { * * @param transaction Execute this query as part of an existing transaction. * @param statement The SQL statement to execute. - * @param opts (optional) The `QueryOptions` to use for this call. If given, + * @param opts (optional) The `Options` to use for this call. If given, * these will take precedence over the options set at the client and * environment levels. * * @par Example: * @snippet samples.cc analyze-query */ + StatusOr AnalyzeSql(Transaction transaction, + SqlStatement statement, Options opts = {}); + + //@{ + /// Backwards compatibility for `QueryOptions`. StatusOr AnalyzeSql(Transaction transaction, SqlStatement statement, - QueryOptions const& opts = {}); + QueryOptions const& opts) { + return AnalyzeSql(std::move(transaction), std::move(statement), + Options(opts)); + } + StatusOr AnalyzeSql( + Transaction transaction, SqlStatement statement, + std::initializer_list) { + return AnalyzeSql(std::move(transaction), std::move(statement)); + } + //@} /** * Executes a batch of SQL DML statements. This method allows many statements @@ -703,7 +815,7 @@ class Client { * @param statement the SQL statement to execute. Please see the * [spanner documentation][dml-partitioned] for the restrictions on the * SQL statements supported by this function. - * @param opts (optional) The `QueryOptions` to use for this call. If given, + * @param opts (optional) The `Options` to use for this call. If given, * these will take precedence over the options set at the client and * environment levels. * @@ -718,15 +830,25 @@ class Client { * https://cloud.google.com/spanner/docs/transactions#partitioned_dml_transactions * [dml-partitioned]: https://cloud.google.com/spanner/docs/dml-partitioned */ + StatusOr ExecutePartitionedDml(SqlStatement statement, + Options opts = {}); + + //@{ + /// Backwards compatibility for `QueryOptions`. + StatusOr ExecutePartitionedDml( + SqlStatement statement, QueryOptions const& opts) { + return ExecutePartitionedDml(std::move(statement), Options(opts)); + } StatusOr ExecutePartitionedDml( - SqlStatement statement, QueryOptions const& opts = {}); + SqlStatement statement, + std::initializer_list) { + return ExecutePartitionedDml(std::move(statement)); + } + //@} private: - QueryOptions OverlayQueryOptions(QueryOptions const&); - std::shared_ptr conn_; Options opts_; - QueryOptions query_opts_{opts_}; }; /** @@ -801,18 +923,6 @@ std::shared_ptr MakeConnection( GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace spanner - -namespace spanner_internal { -GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN - -spanner::QueryOptions OverlayQueryOptions( - spanner::QueryOptions const& preferred, - spanner::QueryOptions const& fallback, - absl::optional const& optimizer_version_env, - absl::optional const& optimizer_statistics_package_env); - -GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END -} // namespace spanner_internal } // namespace cloud } // namespace google diff --git a/google/cloud/spanner/client_options.cc b/google/cloud/spanner/client_options.cc index e465f3bc68290..e69de29bb2d1d 100644 --- a/google/cloud/spanner/client_options.cc +++ b/google/cloud/spanner/client_options.cc @@ -1,49 +0,0 @@ -// Copyright 2021 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "google/cloud/spanner/client_options.h" -#include "google/cloud/spanner/options.h" - -namespace google { -namespace cloud { -namespace spanner { -GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN - -ClientOptions::operator Options() const { - Options opts; - auto optimizer_version = query_options_.optimizer_version(); - if (optimizer_version) { - opts.set(*optimizer_version); - } - auto optimizer_statistics_package = - query_options_.optimizer_statistics_package(); - if (optimizer_statistics_package) { - opts.set( - *optimizer_statistics_package); - } - auto request_priority = query_options_.request_priority(); - if (request_priority) { - opts.set(*request_priority); - } - auto request_tag = query_options_.request_tag(); - if (request_tag) { - opts.set(*request_tag); - } - return opts; -} - -GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END -} // namespace spanner -} // namespace cloud -} // namespace google diff --git a/google/cloud/spanner/client_options.h b/google/cloud/spanner/client_options.h index 8af5d58ca9e1e..ecd4fdecc7d80 100644 --- a/google/cloud/spanner/client_options.h +++ b/google/cloud/spanner/client_options.h @@ -40,7 +40,7 @@ class ClientOptions { * Convert the `ClientOptions` to the new, recommended way to represent * options of all varieties, `google::cloud::Options`. */ - explicit operator Options() const; + explicit operator Options() const { return Options(query_options_); } /// Returns the `QueryOptions` QueryOptions const& query_options() const { return query_options_; } diff --git a/google/cloud/spanner/client_test.cc b/google/cloud/spanner/client_test.cc index fa74db45b4c02..9c8db3d44153b 100644 --- a/google/cloud/spanner/client_test.cc +++ b/google/cloud/spanner/client_test.cc @@ -1098,113 +1098,6 @@ TEST(ClientTest, ProfileQueryWithOptionsSuccess) { EXPECT_EQ(expected_stats, *actual_stats); } -TEST(ClientTest, QueryOptionsOverlayPrecedence) { - // Check optimizer_version. - { - QueryOptions preferred; - preferred.set_optimizer_version("preferred"); - QueryOptions fallback; - fallback.set_optimizer_version("fallback"); - absl::optional optimizer_version_env = "environment"; - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, optimizer_version_env, absl::nullopt) - .optimizer_version(), - "preferred"); - preferred.set_optimizer_version(absl::nullopt); - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, optimizer_version_env, absl::nullopt) - .optimizer_version(), - "fallback"); - fallback.set_optimizer_version(absl::nullopt); - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, optimizer_version_env, absl::nullopt) - .optimizer_version(), - "environment"); - optimizer_version_env = absl::nullopt; - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, optimizer_version_env, absl::nullopt) - .optimizer_version(), - absl::nullopt); - } - - // Check optimizer_statistics_package. - { - QueryOptions preferred; - preferred.set_optimizer_statistics_package("preferred"); - QueryOptions fallback; - fallback.set_optimizer_statistics_package("fallback"); - absl::optional optimizer_statistics_package_env = - "environment"; - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, absl::nullopt, - optimizer_statistics_package_env) - .optimizer_statistics_package(), - "preferred"); - preferred.set_optimizer_statistics_package(absl::nullopt); - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, absl::nullopt, - optimizer_statistics_package_env) - .optimizer_statistics_package(), - "fallback"); - fallback.set_optimizer_statistics_package(absl::nullopt); - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, absl::nullopt, - optimizer_statistics_package_env) - .optimizer_statistics_package(), - "environment"); - optimizer_statistics_package_env = absl::nullopt; - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, absl::nullopt, - optimizer_statistics_package_env) - .optimizer_statistics_package(), - absl::nullopt); - } - - // Check request_priority. - { - QueryOptions preferred; - preferred.set_request_priority(RequestPriority::kHigh); - QueryOptions fallback; - fallback.set_request_priority(RequestPriority::kLow); - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, absl::nullopt, absl::nullopt) - .request_priority(), - RequestPriority::kHigh); - preferred.set_request_priority(absl::nullopt); - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, absl::nullopt, absl::nullopt) - .request_priority(), - RequestPriority::kLow); - fallback.set_request_priority(absl::nullopt); - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, absl::nullopt, absl::nullopt) - .request_priority(), - absl::nullopt); - } - - // Check request_tag. - { - QueryOptions preferred; - preferred.set_request_tag("preferred"); - QueryOptions fallback; - fallback.set_request_tag("fallback"); - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, absl::nullopt, absl::nullopt) - .request_tag(), - "preferred"); - preferred.set_request_tag(absl::nullopt); - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, absl::nullopt, absl::nullopt) - .request_tag(), - "fallback"); - fallback.set_request_tag(absl::nullopt); - EXPECT_EQ(spanner_internal::OverlayQueryOptions( - preferred, fallback, absl::nullopt, absl::nullopt) - .request_tag(), - absl::nullopt); - } -} - struct StringOption { using Type = std::string; }; diff --git a/google/cloud/spanner/google_cloud_cpp_spanner.bzl b/google/cloud/spanner/google_cloud_cpp_spanner.bzl index 7a453994240d7..98d0786b196ba 100644 --- a/google/cloud/spanner/google_cloud_cpp_spanner.bzl +++ b/google/cloud/spanner/google_cloud_cpp_spanner.bzl @@ -137,7 +137,6 @@ google_cloud_cpp_spanner_srcs = [ "backup.cc", "bytes.cc", "client.cc", - "client_options.cc", "commit_options.cc", "connection.cc", "connection_options.cc", diff --git a/google/cloud/spanner/internal/defaults.cc b/google/cloud/spanner/internal/defaults.cc index 1c0e818c52c8d..1a0fd75c0566c 100644 --- a/google/cloud/spanner/internal/defaults.cc +++ b/google/cloud/spanner/internal/defaults.cc @@ -16,6 +16,7 @@ #include "google/cloud/spanner/internal/session_pool.h" #include "google/cloud/spanner/options.h" #include "google/cloud/grpc_options.h" +#include "google/cloud/internal/getenv.h" #include "google/cloud/internal/populate_common_options.h" #include "google/cloud/internal/populate_grpc_options.h" #include "google/cloud/options.h" @@ -58,6 +59,18 @@ Options DefaultOptions(Options opts) { std::chrono::milliseconds(100), std::chrono::minutes(1), kBackoffScaling)); } + if (!opts.has()) { + auto e = internal::GetEnv("SPANNER_OPTIMIZER_VERSION"); + if (e && !e->empty()) { + opts.set(*std::move(e)); + } + } + if (!opts.has()) { + auto e = internal::GetEnv("SPANNER_OPTIMIZER_STATISTICS_PACKAGE"); + if (e && !e->empty()) { + opts.set(*std::move(e)); + } + } // Sets Spanner-specific options from session_pool_options.h if (!opts.has()) { diff --git a/google/cloud/spanner/query_options.cc b/google/cloud/spanner/query_options.cc index 108cb3cde7b5f..ce682dabfea1a 100644 --- a/google/cloud/spanner/query_options.cc +++ b/google/cloud/spanner/query_options.cc @@ -36,6 +36,20 @@ QueryOptions::QueryOptions(Options const& opts) { } } +QueryOptions::operator Options() const { + Options opts; + if (optimizer_version_) { + opts.set(*optimizer_version_); + } + if (optimizer_statistics_package_) { + opts.set( + *optimizer_statistics_package_); + } + if (request_priority_) opts.set(*request_priority_); + if (request_tag_) opts.set(*request_tag_); + return opts; +} + GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace spanner } // namespace cloud diff --git a/google/cloud/spanner/query_options.h b/google/cloud/spanner/query_options.h index 1d3e56fe4103c..6d50693646fd3 100644 --- a/google/cloud/spanner/query_options.h +++ b/google/cloud/spanner/query_options.h @@ -43,11 +43,17 @@ class QueryOptions { QueryOptions& operator=(QueryOptions&&) = default; /** - * Construct from the the new, recommended way to represent options of all - * varieties, `google::cloud::Options`. + * Constructs from the the new, recommended way to represent options + * of all varieties, `google::cloud::Options`. */ explicit QueryOptions(Options const& opts); + /** + * Converts to the new, recommended way to represent options of all + * varieties, `google::cloud::Options`. + */ + explicit operator Options() const; + /// Returns the optimizer version absl::optional const& optimizer_version() const { return optimizer_version_; @@ -112,8 +118,6 @@ class QueryOptions { } private: - // Note: If you add an attribute here, remember to update the implementation - // of Client::OverlayQueryOptions(). absl::optional optimizer_version_; absl::optional optimizer_statistics_package_; absl::optional request_priority_; diff --git a/google/cloud/spanner/query_options_test.cc b/google/cloud/spanner/query_options_test.cc index 053791e8f3bfb..d5389a76220b0 100644 --- a/google/cloud/spanner/query_options_test.cc +++ b/google/cloud/spanner/query_options_test.cc @@ -110,6 +110,23 @@ TEST(QueryOptionsTest, FromOptionsFull) { EXPECT_EQ(*query_opts.request_tag(), "tag"); } +TEST(QueryOptionsTest, OptionsRoundTrip) { + auto const query_opts = QueryOptions{} + .set_optimizer_version("1") + .set_optimizer_statistics_package("latest") + .set_request_priority(RequestPriority::kHigh) + .set_request_tag("tag"); + QueryOptions const rt_query_opts(Options{query_opts}); + ASSERT_TRUE(rt_query_opts.optimizer_version()); + EXPECT_EQ(*rt_query_opts.optimizer_version(), "1"); + ASSERT_TRUE(rt_query_opts.optimizer_statistics_package()); + EXPECT_EQ(*rt_query_opts.optimizer_statistics_package(), "latest"); + ASSERT_TRUE(rt_query_opts.request_priority()); + EXPECT_EQ(*rt_query_opts.request_priority(), RequestPriority::kHigh); + ASSERT_TRUE(rt_query_opts.request_tag()); + EXPECT_EQ(*rt_query_opts.request_tag(), "tag"); +} + } // namespace GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace spanner diff --git a/google/cloud/spanner/samples/samples.cc b/google/cloud/spanner/samples/samples.cc index 88fc45aada235..22ca1bca977bb 100644 --- a/google/cloud/spanner/samples/samples.cc +++ b/google/cloud/spanner/samples/samples.cc @@ -1856,7 +1856,7 @@ void QueryDataWithTimestamp(google::cloud::spanner::Client client) { std::tuple, absl::optional>; - auto rows = client.ExecuteQuery(select); + auto rows = client.ExecuteQuery(std::move(select)); for (auto const& row : spanner::StreamOf(rows)) { if (!row) throw std::runtime_error(row.status().message()); std::cout << std::get<0>(*row) << " " << std::get<1>(*row); @@ -1944,7 +1944,7 @@ void QueryWithJsonParameter(google::cloud::spanner::Client client) { {{"details", spanner::Value(std::move(rating9_details))}}); using RowType = std::tuple>; - auto rows = client.ExecuteQuery(select); + auto rows = client.ExecuteQuery(std::move(select)); for (auto const& row : spanner::StreamOf(rows)) { if (!row) throw std::runtime_error(row.status().message()); std::cout << "VenueId: " << std::get<0>(*row) << ", "; @@ -2007,7 +2007,7 @@ void QueryWithNumericParameter(google::cloud::spanner::Client client) { {{"revenue", spanner::Value(std::move(revenue))}}); using RowType = std::tuple>; - auto rows = client.ExecuteQuery(select); + auto rows = client.ExecuteQuery(std::move(select)); for (auto const& row : spanner::StreamOf(rows)) { if (!row) throw std::runtime_error(row.status().message()); std::cout << "VenueId: " << std::get<0>(*row) << "\t"; @@ -2070,10 +2070,10 @@ void SetTransactionTag(google::cloud::spanner::Client client) { " WHERE OutdoorVenue = false"); // Sets the request tag to "app=concert,env=dev,action=update". // This will only be set on this request. - auto update = - client.ExecuteDml(txn, update_statement, - spanner::QueryOptions().set_request_tag( - "app=concert,env=dev,action=update")); + auto update = client.ExecuteDml( + txn, std::move(update_statement), + google::cloud::Options{}.set( + "app=concert,env=dev,action=update")); if (!update) return std::move(update).status(); spanner::SqlStatement insert_statement( @@ -2089,10 +2089,10 @@ void SetTransactionTag(google::cloud::spanner::Client client) { }); // Sets the request tag to "app=concert,env=dev,action=insert". // This will only be set on this request. - auto insert = - client.ExecuteDml(txn, insert_statement, - spanner::QueryOptions().set_request_tag( - "app=concert,env=dev,action=select")); + auto insert = client.ExecuteDml( + txn, std::move(insert_statement), + google::cloud::Options{}.set( + "app=concert,env=dev,action=select")); if (!insert) return std::move(insert).status(); return spanner::Mutations{}; }, @@ -2105,13 +2105,13 @@ void SetTransactionTag(google::cloud::spanner::Client client) { void ReadStaleData(google::cloud::spanner::Client client) { namespace spanner = ::google::cloud::spanner; auto opts = spanner::Transaction::ReadOnlyOptions(std::chrono::seconds(15)); - auto read_only = spanner::MakeReadOnlyTransaction(opts); + auto read_only = spanner::MakeReadOnlyTransaction(std::move(opts)); spanner::SqlStatement select( "SELECT SingerId, AlbumId, AlbumTitle FROM Albums"); using RowType = std::tuple; - auto rows = client.ExecuteQuery(read_only, select); + auto rows = client.ExecuteQuery(std::move(read_only), std::move(select)); for (auto const& row : spanner::StreamOf(rows)) { if (!row) throw std::runtime_error(row.status().message()); std::cout << "SingerId: " << std::get<0>(*row) @@ -2128,9 +2128,9 @@ void SetRequestTag(google::cloud::spanner::Client client) { "SELECT SingerId, AlbumId, AlbumTitle FROM Albums"); using RowType = std::tuple; - auto opts = spanner::QueryOptions().set_request_tag( + auto opts = google::cloud::Options{}.set( "app=concert,env=dev,action=select"); - auto rows = client.ExecuteQuery(select, opts); + auto rows = client.ExecuteQuery(std::move(select), std::move(opts)); for (auto const& row : spanner::StreamOf(rows)) { if (!row) throw std::runtime_error(row.status().message()); std::cout << "SingerId: " << std::get<0>(*row) @@ -2149,7 +2149,8 @@ void UsePartitionQuery(google::cloud::spanner::Client client) { "SELECT SingerId, FirstName, LastName FROM Singers"); using RowType = std::tuple; - auto partitions = client.PartitionQuery(std::move(txn), select, {}); + auto partitions = + client.PartitionQuery(std::move(txn), std::move(select), {}); if (!partitions) throw std::runtime_error(partitions.status().message()); int number_of_rows = 0; for (auto const& partition : *partitions) { @@ -2267,10 +2268,10 @@ void CreateClientWithQueryOptions(std::string const& project_id, namespace spanner = ::google::cloud::spanner; spanner::Client client( spanner::MakeConnection(db), - spanner::ClientOptions().set_query_options( - spanner::QueryOptions() - .set_optimizer_version("1") - .set_optimizer_statistics_package("auto_20191128_14_47_22UTC"))); + google::cloud::Options{} + .set("1") + .set( + "auto_20191128_14_47_22UTC")); //! [END spanner_create_client_with_query_options] } @@ -2287,9 +2288,10 @@ void CreateClientWithQueryOptionsCommand(std::vector argv) { void QueryWithQueryOptions(google::cloud::spanner::Client client) { namespace spanner = ::google::cloud::spanner; auto sql = spanner::SqlStatement("SELECT SingerId, FirstName FROM Singers"); - auto opts = spanner::QueryOptions() - .set_optimizer_version("1") - .set_optimizer_statistics_package("latest"); + auto opts = + google::cloud::Options{} + .set("1") + .set("latest"); auto rows = client.ExecuteQuery(std::move(sql), std::move(opts)); using RowType = std::tuple;