Skip to content

Commit

Permalink
fix(spanner): switch order of ListBackupOperations() filter conjuncts (
Browse files Browse the repository at this point in the history
…#7746)

Check that the metadata is of the desired type in the left-hand
side of the `ListBackupOperations()` filter AND expression, before
looking at its value.

This appears to be a recent change in behavior for the production
service, which previously accepted the original ordering, but now
would say: "INVALID_ARGUMENT: Invalid ListBackupOperations request."

Also change the filter subexpressions to be exact matches, rather
than just "contains", so as to make the filtering more precise.

Finally, take this opportunity to log the name of running sample,
which makes it easier to delineate the RPC log.

Fixes #7741.
  • Loading branch information
devbww authored Dec 16, 2021
1 parent eeb02b9 commit 7315ef8
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,9 @@ TEST_F(BackupIntegrationTest, BackupRestore) {

// List the backup operations
std::ostringstream backup_op_filter;
backup_op_filter << "(metadata.database:" << db.database_id() << ") AND "
<< "(metadata.@type:type.googleapis.com/"
<< "google.spanner.admin.database.v1.CreateBackupMetadata)";
backup_op_filter << "(metadata.@type=type.googleapis.com/"
<< "google.spanner.admin.database.v1.CreateBackupMetadata)"
<< " AND (metadata.database=" << db.FullName() << ")";
google::spanner::admin::database::v1::ListBackupOperationsRequest lreq;
lreq.set_parent(in.FullName());
lreq.set_filter(backup_op_filter.str());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ TEST_F(BackupIntegrationTest, BackupRestore) {

// List the backup operations
std::ostringstream backup_op_filter;
backup_op_filter << "(metadata.database:" << db.database_id() << ") AND "
<< "(metadata.@type:type.googleapis.com/"
<< "google.spanner.admin.database.v1.CreateBackupMetadata)";
backup_op_filter << "(metadata.@type=type.googleapis.com/"
<< "google.spanner.admin.database.v1.CreateBackupMetadata)"
<< " AND (metadata.database=" << db.FullName() << ")";
std::vector<std::string> db_names;
for (auto const& operation : database_admin_client_.ListBackupOperations(
in, backup_op_filter.str())) {
Expand Down
8 changes: 5 additions & 3 deletions google/cloud/spanner/samples/samples.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1138,11 +1138,12 @@ void ListBackupOperations(
std::string const& project_id, std::string const& instance_id,
std::string const& database_id) {
google::cloud::spanner::Instance in(project_id, instance_id);
google::cloud::spanner::Database database(in, database_id);
google::spanner::admin::database::v1::ListBackupOperationsRequest request;
request.set_parent(in.FullName());
request.set_filter(std::string("(metadata.database:") + database_id +
") AND " + "(metadata.@type:type.googleapis.com/" +
"google.spanner.admin.database.v1.CreateBackupMetadata)");
request.set_filter(std::string("(metadata.@type=type.googleapis.com/") +
"google.spanner.admin.database.v1.CreateBackupMetadata)" +
" AND (metadata.database=" + database.FullName() + ")");
for (auto const& operation : client.ListBackupOperations(request)) {
if (!operation) throw std::runtime_error(operation.status().message());
google::spanner::admin::database::v1::CreateBackupMetadata metadata;
Expand Down Expand Up @@ -3589,6 +3590,7 @@ void SampleBanner(std::string const& name) {
<< absl::FormatTime("%Y-%m-%dT%H:%M:%SZ", absl::Now(),
absl::UTCTimeZone())
<< std::endl;
GCP_LOG(DEBUG) << "Running " << name << " sample";
}

std::string PickConfig(google::cloud::spanner_admin::InstanceAdminClient client,
Expand Down

0 comments on commit 7315ef8

Please sign in to comment.