Skip to content

Commit

Permalink
[#23304] xCluster: fix ysql_dump/Postgres so pg_class OIDs are preserved
Browse files Browse the repository at this point in the history
Summary:
This diff modifies ysql_dump and Postgres so all pg_class and pg_type OID are preserved across a backup/restore when the (existing) --include-yb-metadata flag is passed to ysql_dump.

See #23304 for context on why we want to do this.
Jira: DB-12226

Test Plan:
Jenkins

```
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpAllWithYbMetadata' >& /tmp/generic.mdl.log
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpWithYbMetadata' >& /tmp/generic.mdl.log
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpColocatedDB' >& /tmp/generic.mdl.log
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpLegacyColocatedDB' >& /tmp/generic.mdl.log
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpWithoutYbMetadata' >& /tmp/generic.mdl.log
```

Added two new tests to verify OIDs are preserved properly:
```
ybd --cxx-test yb-backup-cross-feature-test --gtest_filter '*.TestPreservingPgEnumOids' --test-args '--verbose_yb_backup' >& /tmp/generic.mdl.log
ybd --cxx-test yb-backup-cross-feature-test --gtest_filter '*.TestPreservingPgTypeAndClassOids' --test-args '--verbose_yb_backup' >& /tmp/generic.mdl.log
```

Reviewers: myang

Reviewed By: myang

Subscribers: yql, ybase

Differential Revision: https://phorge.dev.yugabyte.com/D36675
  • Loading branch information
mdbridge committed Aug 17, 2024
1 parent 68ac66e commit 516ead0
Show file tree
Hide file tree
Showing 9 changed files with 663 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/postgres/src/backend/catalog/heap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ heap_create_with_catalog(const char *relname,
if (!OidIsValid(relid))
{
/* Use binary-upgrade override for pg_class.oid/relfilenode? */
if (IsBinaryUpgrade && !yb_binary_restore &&
if ((IsBinaryUpgrade || yb_binary_restore) &&
(relkind == RELKIND_RELATION || relkind == RELKIND_SEQUENCE ||
relkind == RELKIND_VIEW || relkind == RELKIND_MATVIEW ||
relkind == RELKIND_COMPOSITE_TYPE || relkind == RELKIND_FOREIGN_TABLE ||
Expand All @@ -1284,7 +1284,7 @@ heap_create_with_catalog(const char *relname,
binary_upgrade_next_heap_pg_class_oid = InvalidOid;
}
/* There might be no TOAST table, so we have to test for it. */
else if (IsBinaryUpgrade && !yb_binary_restore &&
else if ((IsBinaryUpgrade || yb_binary_restore) &&
OidIsValid(binary_upgrade_next_toast_pg_class_oid) &&
relkind == RELKIND_TOASTVALUE)
{
Expand Down
2 changes: 1 addition & 1 deletion src/postgres/src/backend/catalog/index.c
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ index_create(Relation heapRelation,
if (!OidIsValid(indexRelationId))
{
/* Use binary-upgrade override for pg_class.oid/relfilenode? */
if (IsBinaryUpgrade && !yb_binary_restore)
if (IsBinaryUpgrade || yb_binary_restore)
{
if (!OidIsValid(binary_upgrade_next_index_pg_class_oid))
ereport(ERROR,
Expand Down
29 changes: 19 additions & 10 deletions src/postgres/src/bin/pg_dump/pg_dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -11381,8 +11381,7 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
binary_upgrade_set_type_oids_by_type_oid(fout, q,
tyinfo->dobj.catId.oid,
false);
if (dopt->binary_upgrade)
binary_upgrade_set_pg_class_oids(fout, q, tyinfo->typrelid, false);
binary_upgrade_set_pg_class_oids(fout, q, tyinfo->typrelid, false);
}

qtypname = pg_strdup(fmtId(tyinfo->dobj.name));
Expand Down Expand Up @@ -15946,7 +15945,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)

appendPQExpBuffer(delq, "DROP VIEW %s;\n", qualrelname);

if (dopt->binary_upgrade)
if (dopt->binary_upgrade || dopt->include_yb_metadata)
binary_upgrade_set_pg_class_oids(fout, q,
tbinfo->dobj.catId.oid, false);

Expand Down Expand Up @@ -16023,10 +16022,21 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)

appendPQExpBuffer(delq, "DROP %s %s;\n", reltypename, qualrelname);

if (dopt->binary_upgrade)
if (dopt->binary_upgrade || dopt->include_yb_metadata)
{
binary_upgrade_set_pg_class_oids(fout, q,
tbinfo->dobj.catId.oid, false);
/*
* We may create a primary key index as part of the CREATE TABLE
* statement we generate here; accordingly, set things up so we
* will set its OID correctly in binary update mode.
*/
if (tbinfo->primaryKeyIndex)
{
IndxInfo *index = tbinfo->primaryKeyIndex;
binary_upgrade_set_pg_class_oids(fout, q,
index->dobj.catId.oid, true);
}
}

/* Get the table properties from YB, if relevant. */
Expand Down Expand Up @@ -16223,7 +16233,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
bool parent_has_primary_key = false;
if (tbinfo->ispartition)
{
TableInfo *parentRel = tbinfo->parents[0];
TableInfo *parentRel = parents[0];
parent_has_primary_key = parentRel->primaryKeyIndex;
}

Expand Down Expand Up @@ -16905,7 +16915,7 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
int nstatcols;
int nstatvals;

if (dopt->binary_upgrade)
if (dopt->binary_upgrade || dopt->include_yb_metadata)
binary_upgrade_set_pg_class_oids(fout, q,
indxinfo->dobj.catId.oid, true);

Expand Down Expand Up @@ -17131,7 +17141,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
exit_horribly(NULL, "missing index for constraint \"%s\"\n",
coninfo->dobj.name);

if (dopt->binary_upgrade)
if (dopt->binary_upgrade || dopt->include_yb_metadata)
binary_upgrade_set_pg_class_oids(fout, q,
indxinfo->dobj.catId.oid, true);

Expand Down Expand Up @@ -17633,9 +17643,8 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)

if (dopt->binary_upgrade || dopt->include_yb_metadata)
{
if (dopt->binary_upgrade)
binary_upgrade_set_pg_class_oids(fout, query,
tbinfo->dobj.catId.oid, false);
binary_upgrade_set_pg_class_oids(fout, query,
tbinfo->dobj.catId.oid, false);
binary_upgrade_set_type_oids_by_rel_oid(fout, query,
tbinfo->dobj.catId.oid);
}
Expand Down
Loading

0 comments on commit 516ead0

Please sign in to comment.