Skip to content

Commit

Permalink
[BACKPORT pg15-cherrypicks][#23304] fix Postgres so old dumps can be …
Browse files Browse the repository at this point in the history
…loaded that do not have pg_class OIDs

Summary:
Original commit: 16941de / D37360

Modifications due to standard conflicts that auto resolve could not resolve:

- heap.c
  - function heap_create_with_catalog
    - master commit 16941de changes the logic for when to require pg_class heap OIDs (they are now only required when yb_ignore_heap_pg_class_oids is false when in yb_binary_restore mode)
    - PG cherry picks trunk via merge of master commit 516ead0 with upstream Postgres 37b2764593c073ca61c2baebd7d85666e553928b refactors how RELKIND are handled
    - resolution: apply the just the intended logic change to the upstream Postgres code

Rest of original diff summary:

The previous diff of this stack, https://phorge.dev.yugabyte.com/D36675, changed ysql_dump/Postgres so it emits information about the pg_class OIDs of objects and at restore time verifies that every object that need such an OID has one provided.

Unfortunately, this breaks backward compatibility: if we attempt to restore a dump made before that diff, it fails because those dumps don't have information about every object that needs such OID.

This diff fixes this by making the check only applied if a new GUC, yb_ignore_heap_pg_class_oids, is turned on.  The new dump sets this variable in its output but it would obviously not be set in the old dump output.

This let's us restore old backups without problem but would turn off the check for some cases we don't use (e.g., trying to restore output of pg_dump with binary upgrade set).

Test Plan:
Verify preserving all pg_class OIDs works for new version:
```
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
```

Other tests:
```
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpAllWithYbMetadata' >& /tmp/generic.mdl.log
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpWithYbMetadata' >& /tmp/generic.mdl.log2
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpColocatedDB' >& /tmp/generic.mdl.log3
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpLegacyColocatedDB' >& /tmp/generic.mdl.log4
ybd  --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpWithoutYbMetadata' >& /tmp/generic.mdl.log5
```

I did not verify that backwards compatibility was maintained -- being able to import a PG 11 dump into PG 15 will likely require more than just this diff.

Reviewers: jason, tfoucher

Reviewed By: jason

Differential Revision: https://phorge.dev.yugabyte.com/D37810
  • Loading branch information
mdbridge committed Sep 6, 2024
1 parent a88dc15 commit 230591b
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/postgres/src/backend/catalog/heap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1319,8 +1319,11 @@ heap_create_with_catalog(const char *relname,
*/
if (!OidIsValid(relid))
{
bool heap_pg_class_oids_supplied = IsBinaryUpgrade && !yb_binary_restore;
if (yb_binary_restore && !yb_ignore_heap_pg_class_oids)
heap_pg_class_oids_supplied = true;
/* Use binary-upgrade override for pg_class.oid and relfilenode */
if (IsBinaryUpgrade || yb_binary_restore)
if (heap_pg_class_oids_supplied)
{
/*
* Indexes are not supported here; they use
Expand Down
11 changes: 11 additions & 0 deletions src/postgres/src/backend/utils/misc/guc.c
Original file line number Diff line number Diff line change
Expand Up @@ -2550,6 +2550,17 @@ static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},

{
{"yb_ignore_heap_pg_class_oids", PGC_SUSET, DEVELOPER_OPTIONS,
gettext_noop("Ignores requests to set heap pg_class OIDs in yb_binary_restore mode"),
NULL,
GUC_NOT_IN_SAMPLE
},
&yb_ignore_heap_pg_class_oids,
true,
NULL, NULL, NULL
},

{
{"yb_test_system_catalogs_creation", PGC_SUSET, DEVELOPER_OPTIONS,
gettext_noop("Relaxes some internal sanity checks for system "
Expand Down
1 change: 1 addition & 0 deletions src/postgres/src/bin/pg_dump/pg_backup_archiver.c
Original file line number Diff line number Diff line change
Expand Up @@ -3079,6 +3079,7 @@ _doSetFixedOutputState(ArchiveHandle *AH)
if (AH->public.dopt->include_yb_metadata)
{
ahprintf(AH, "SET yb_binary_restore = true;\n");
ahprintf(AH, "SET yb_ignore_heap_pg_class_oids = false;\n");
ahprintf(AH, "SET yb_non_ddl_txn_for_sys_tables_allowed = true;\n");
}
ahprintf(AH, "SET statement_timeout = 0;\n");
Expand Down
1 change: 1 addition & 0 deletions src/postgres/src/test/regress/data/yb_ysql_dump.data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
-- Dumped by ysql_dump version 15.2-YB-2.23.1.1505-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
-- Dumped by ysql_dump version 15.2-YB-2.23.1.1505-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
-- Dumped by ysql_dump version 15.2-YB-2.23.1.1505-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down
6 changes: 6 additions & 0 deletions src/postgres/src/test/regress/data/yb_ysql_dumpall.data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ ALTER ROLE yugabyte_test WITH SUPERUSER INHERIT CREATEROLE CREATEDB LOGIN NOREPL
-- Dumped by ysql_dump version 15.2-YB-2.23.1.1505-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down Expand Up @@ -228,6 +229,7 @@ SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);
-- Dumped by ysql_dump version 15.2-YB-2.23.1.1505-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down Expand Up @@ -301,6 +303,7 @@ SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);
-- Dumped by ysql_dump version 15.2-YB-2.23.1.1505-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down Expand Up @@ -339,6 +342,7 @@ CREATE DATABASE system_platform WITH TEMPLATE = template0 ENCODING = 'UTF8' LOCA
\connect system_platform

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down Expand Up @@ -407,6 +411,7 @@ SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);
-- Dumped by ysql_dump version 15.2-YB-2.23.1.1505-b0

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down Expand Up @@ -445,6 +450,7 @@ CREATE DATABASE yugabyte WITH TEMPLATE = template0 ENCODING = 'UTF8' LOCALE_PROV
\connect yugabyte

SET yb_binary_restore = true;
SET yb_ignore_heap_pg_class_oids = false;
SET yb_non_ddl_txn_for_sys_tables_allowed = true;
SET statement_timeout = 0;
SET lock_timeout = 0;
Expand Down
2 changes: 2 additions & 0 deletions src/yb/yql/pggate/util/yb_guc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ bool suppress_nonpg_logs = false;

bool yb_binary_restore = false;

bool yb_ignore_heap_pg_class_oids = true;

bool yb_pushdown_strict_inequality = true;

bool yb_pushdown_is_not_null = true;
Expand Down
7 changes: 7 additions & 0 deletions src/yb/yql/pggate/util/yb_guc.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ extern int ysql_max_in_flight_ops;
*/
extern bool yb_binary_restore;

/*
* Guc variable for ignoring requests to set heap pg_class oids when yb_binary_restore is set.
*
* If true then calls to pg_catalog.binary_upgrade_set_next_heap_pg_class_oid will have no effect.
*/
extern bool yb_ignore_heap_pg_class_oids;

/*
* Set to true only for runs with EXPLAIN ANALYZE
*/
Expand Down
7 changes: 7 additions & 0 deletions src/yb/yql/pggate/util/ybc_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ extern int yb_locks_txn_locks_per_tablet;
*/
extern bool yb_binary_restore;

/*
* Guc variable for ignoring requests to set heap pg_class oids when yb_binary_restore is set.
*
* If true then calls to pg_catalog.binary_upgrade_set_next_heap_pg_class_oid will have no effect.
*/
extern bool yb_ignore_heap_pg_class_oids;

/*
* Set to true only for runs with EXPLAIN ANALYZE
*/
Expand Down

0 comments on commit 230591b

Please sign in to comment.