From 230591b21bff194e5abf2f2da742d57a64c81eca Mon Sep 17 00:00:00 2001 From: Mark Lillibridge Date: Fri, 16 Aug 2024 10:13:52 -0700 Subject: [PATCH] [BACKPORT pg15-cherrypicks][#23304] fix Postgres so old dumps can be loaded that do not have pg_class OIDs Summary: Original commit: 16941de77d / D37360 Modifications due to standard conflicts that auto resolve could not resolve: - heap.c - function heap_create_with_catalog - master commit 16941de77d0fb5a1bbc4e0bf2e7a8795a3c8e7a1 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 516ead043e2446cb055313195d515e72859daf16 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 --- src/postgres/src/backend/catalog/heap.c | 5 ++++- src/postgres/src/backend/utils/misc/guc.c | 11 +++++++++++ src/postgres/src/bin/pg_dump/pg_backup_archiver.c | 1 + .../src/test/regress/data/yb_ysql_dump.data.sql | 1 + .../data/yb_ysql_dump_colocated_database.data.sql | 1 + .../yb_ysql_dump_legacy_colocated_database.data.sql | 1 + .../src/test/regress/data/yb_ysql_dumpall.data.sql | 6 ++++++ src/yb/yql/pggate/util/yb_guc.cc | 2 ++ src/yb/yql/pggate/util/yb_guc.h | 7 +++++++ src/yb/yql/pggate/util/ybc_util.h | 7 +++++++ 10 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/postgres/src/backend/catalog/heap.c b/src/postgres/src/backend/catalog/heap.c index ad3ed94bc54a..8f4efe36447f 100644 --- a/src/postgres/src/backend/catalog/heap.c +++ b/src/postgres/src/backend/catalog/heap.c @@ -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 diff --git a/src/postgres/src/backend/utils/misc/guc.c b/src/postgres/src/backend/utils/misc/guc.c index 44e13fceceb0..c1d9212cb36d 100644 --- a/src/postgres/src/backend/utils/misc/guc.c +++ b/src/postgres/src/backend/utils/misc/guc.c @@ -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 " diff --git a/src/postgres/src/bin/pg_dump/pg_backup_archiver.c b/src/postgres/src/bin/pg_dump/pg_backup_archiver.c index ab481117a470..33716bf8ac88 100644 --- a/src/postgres/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/postgres/src/bin/pg_dump/pg_backup_archiver.c @@ -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"); diff --git a/src/postgres/src/test/regress/data/yb_ysql_dump.data.sql b/src/postgres/src/test/regress/data/yb_ysql_dump.data.sql index bdd98402dd4b..a079d5b05406 100644 --- a/src/postgres/src/test/regress/data/yb_ysql_dump.data.sql +++ b/src/postgres/src/test/regress/data/yb_ysql_dump.data.sql @@ -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; diff --git a/src/postgres/src/test/regress/data/yb_ysql_dump_colocated_database.data.sql b/src/postgres/src/test/regress/data/yb_ysql_dump_colocated_database.data.sql index 844af75f439e..55f238098a1b 100644 --- a/src/postgres/src/test/regress/data/yb_ysql_dump_colocated_database.data.sql +++ b/src/postgres/src/test/regress/data/yb_ysql_dump_colocated_database.data.sql @@ -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; diff --git a/src/postgres/src/test/regress/data/yb_ysql_dump_legacy_colocated_database.data.sql b/src/postgres/src/test/regress/data/yb_ysql_dump_legacy_colocated_database.data.sql index b2fb8f681b08..9a9204bf770b 100644 --- a/src/postgres/src/test/regress/data/yb_ysql_dump_legacy_colocated_database.data.sql +++ b/src/postgres/src/test/regress/data/yb_ysql_dump_legacy_colocated_database.data.sql @@ -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; diff --git a/src/postgres/src/test/regress/data/yb_ysql_dumpall.data.sql b/src/postgres/src/test/regress/data/yb_ysql_dumpall.data.sql index ee9e2661356a..52f07da57a1c 100644 --- a/src/postgres/src/test/regress/data/yb_ysql_dumpall.data.sql +++ b/src/postgres/src/test/regress/data/yb_ysql_dumpall.data.sql @@ -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; @@ -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; @@ -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; @@ -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; @@ -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; @@ -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; diff --git a/src/yb/yql/pggate/util/yb_guc.cc b/src/yb/yql/pggate/util/yb_guc.cc index 6a68ba67c4d9..24c2968901c7 100644 --- a/src/yb/yql/pggate/util/yb_guc.cc +++ b/src/yb/yql/pggate/util/yb_guc.cc @@ -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; diff --git a/src/yb/yql/pggate/util/yb_guc.h b/src/yb/yql/pggate/util/yb_guc.h index 1fe91886debe..c13a9e120751 100644 --- a/src/yb/yql/pggate/util/yb_guc.h +++ b/src/yb/yql/pggate/util/yb_guc.h @@ -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 */ diff --git a/src/yb/yql/pggate/util/ybc_util.h b/src/yb/yql/pggate/util/ybc_util.h index 2a4591b7ba99..4fa9ee276d6b 100644 --- a/src/yb/yql/pggate/util/ybc_util.h +++ b/src/yb/yql/pggate/util/ybc_util.h @@ -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 */