Skip to content

Commit

Permalink
[#11691] xCluster: (Fix) Do not return error when schema name not fou…
Browse files Browse the repository at this point in the history
…nd for YSQL tables

Summary:
This is a fix for an issue introduced by the previous  diff https://phabricator.dev.yugabyte.com/D17099

Issue: https://yugabyte.slack.com/archives/C03G3F2H4AU/p1654647033631909

It appears that it is possible that some YSQL tables' schema names can not be found even in pg system tables. Previously, an error was raised in this case. The fix is to change the error to a warning message.

Test Plan:
Manually let all non-system YSQL tables passing the two if-clauses in the following block in `CatalogManager::GetTableSchemaInternal`:
```
if (!table->is_system() && l->table_type() == TableType::PGSQL_TABLE_TYPE &&
      !resp->schema().has_pgschema_name()) {
    SharedLock lock(mutex_);
    TRACE("Acquired catalog manager lock for schema name lookup");

    auto pgschema_name = GetPgSchemaName(table);
    if (!pgschema_name.ok() || pgschema_name->empty()) {
      LOG(WARNING) << Format(
          "Unable to find schema name for YSQL table $0.$1 due to error: $2",
          table->namespace_name(), table->name(), pgschema_name.ToString());
    } else {
      resp->mutable_schema()->set_pgschema_name(*pgschema_name);
    }
  }
```
Verify that no error is returned.

Reviewers: jhe

Reviewed By: jhe

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D17563
  • Loading branch information
LambdaWL committed Jun 10, 2022
1 parent 653f44c commit 3427342
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions src/yb/master/catalog_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6091,19 +6091,21 @@ Status CatalogManager::GetTableSchemaInternal(const GetTableSchemaRequestPB* req
resp->mutable_schema()->CopyFrom(l->pb.schema());
}

// Due to pgschema_name being added after 2.13, older tables may not have this field.
// So backfill pgschema_name for non-system YSQL table schema.
if (!table->is_system() && l->table_type() == TableType::PGSQL_TABLE_TYPE &&
!resp->schema().has_pgschema_name()) {
// Due to pgschema_name being added after 2.13, older YSQL tables may not have this field.
// So backfill pgschema_name for older YSQL tables. Skip for some special cases.
if (l->table_type() == TableType::PGSQL_TABLE_TYPE &&
!resp->schema().has_pgschema_name() &&
!table->is_system() &&
!IsSequencesSystemTable(*table) &&
!table->IsColocationParentTable()) {
SharedLock lock(mutex_);
TRACE("Acquired catalog manager lock for schema name lookup");

auto pgschema_name = GetPgSchemaName(table);
if (!pgschema_name.ok() || pgschema_name->empty()) {
Status s = STATUS_SUBSTITUTE(NotFound,
LOG(WARNING) << Format(
"Unable to find schema name for YSQL table $0.$1 due to error: $2",
table->namespace_name(), table->name(), pgschema_name.ToString());
return SetupError(resp->mutable_error(), MasterErrorPB::INVALID_SCHEMA, s);
} else {
resp->mutable_schema()->set_pgschema_name(*pgschema_name);
}
Expand Down

0 comments on commit 3427342

Please sign in to comment.