From 692fe496bdb5fa6dcffeb9f85b6188ceb1df707a Mon Sep 17 00:00:00 2001 From: David Steele Date: Wed, 4 May 2022 08:22:45 -0400 Subject: [PATCH] Remove dependency on pg_database.datlastsysoid. This column has been removed in PostgreSQL 15. Rather than add a lot of special handling, it seems better just to update all versions to not depend on this column. Add centralized functions to identify the type of database (i.e. system or user) by name and use FirstNormalObjectId when a name is not available. The new query in the db module will still return the prior result for PostgreSQL <= 15, which will be stored in the manifest. This is important to preserve behavior when downgrading pgBackRest. There are no concerns here for PostgreSQL 15 since older versions of pgBackRest won't be able to restore backups for PostgreSQL 15 anyway. --- doc/xml/release.xml | 13 +++++++ src/command/info/info.c | 2 +- src/command/restore/restore.c | 5 ++- src/db/db.c | 4 ++- src/info/manifest.h | 2 +- src/postgres/interface.c | 34 ++++++++++++++++++ src/postgres/interface.h | 16 +++++---- src/postgres/interface/static.vendor.h | 46 ++++++++++++++++++++++++ test/define.yaml | 2 +- test/src/common/harnessPq.h | 4 ++- test/src/module/command/infoTest.c | 12 +++---- test/src/module/command/restoreTest.c | 18 +++++----- test/src/module/info/manifestTest.c | 32 ++++++++--------- test/src/module/postgres/interfaceTest.c | 15 ++++++++ 14 files changed, 159 insertions(+), 46 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index aa2a36a13..6f81ab854 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -169,6 +169,19 @@ + + + + + + + + + + +

15 support.

+ + diff --git a/src/command/info/info.c b/src/command/info/info.c index c161a3487..1fe340f50 100644 --- a/src/command/info/info.c +++ b/src/command/info/info.c @@ -474,7 +474,7 @@ backupListAdd( const ManifestDb *db = manifestDb(repoData->manifest, dbIdx); // Do not display template databases - if (db->id > db->lastSystemId) + if (!pgDbIsTemplate(db->name)) { Variant *database = varNewKv(kvNew()); diff --git a/src/command/restore/restore.c b/src/command/restore/restore.c index 576938fd3..5d554efad 100644 --- a/src/command/restore/restore.c +++ b/src/command/restore/restore.c @@ -1346,8 +1346,7 @@ restoreSelectiveExpression(Manifest *manifest) { const ManifestDb *systemDb = manifestDb(manifest, systemDbIdx); - if (strEqZ(systemDb->name, "template0") || strEqZ(systemDb->name, "template1") || - strEqZ(systemDb->name, "postgres") || systemDb->id < PG_USER_OBJECT_MIN_ID) + if (pgDbIsSystem(systemDb->name) || pgDbIsSystemId(systemDb->id)) { // Build the system id list and add to the dbList for logging and checking const String *systemDbId = varStrForce(VARUINT(systemDb->id)); @@ -1366,7 +1365,7 @@ restoreSelectiveExpression(Manifest *manifest) // In the highly unlikely event that a system database was somehow added after the backup began, it will only be // found in the file list and not the manifest db section, so add it to the system database list - if (cvtZToUInt64(strZ(dbId)) < PG_USER_OBJECT_MIN_ID) + if (pgDbIsSystemId(cvtZToUInt(strZ(dbId)))) strLstAddIfMissing(systemDbIdList, dbId); strLstAddIfMissing(dbList, dbId); diff --git a/src/db/db.c b/src/db/db.c index 6d2d83b78..ffbba19d3 100644 --- a/src/db/db.c +++ b/src/db/db.c @@ -581,7 +581,9 @@ dbList(Db *this) PACK, dbQuery( this, pgClientQueryResultAny, - STRDEF("select oid::oid, datname::text, datlastsysoid::oid from pg_catalog.pg_database"))); + STRDEF( + "select oid::oid, datname::text, (select oid::oid from pg_catalog.pg_database where datname = 'template0')" + " as datlastsysoid from pg_catalog.pg_database"))); } /**********************************************************************************************************************************/ diff --git a/src/info/manifest.h b/src/info/manifest.h index 7a495e40a..982463a1b 100644 --- a/src/info/manifest.h +++ b/src/info/manifest.h @@ -87,7 +87,7 @@ typedef struct ManifestDb { const String *name; // Db name (must be first member in struct) unsigned int id; // Db oid - unsigned int lastSystemId; // Highest oid used by system objects in this database + unsigned int lastSystemId; // Highest oid used by system objects (deprecated - do not use) } ManifestDb; /*********************************************************************************************************************************** diff --git a/src/postgres/interface.c b/src/postgres/interface.c index ed1a7aa67..9351b8d57 100644 --- a/src/postgres/interface.c +++ b/src/postgres/interface.c @@ -10,6 +10,7 @@ PostgreSQL Interface #include "common/memContext.h" #include "common/regExp.h" #include "postgres/interface.h" +#include "postgres/interface/static.vendor.h" #include "postgres/interface/version.h" #include "postgres/version.h" #include "storage/helper.h" @@ -239,6 +240,39 @@ pgInterfaceVersion(unsigned int pgVersion) FUNCTION_TEST_RETURN_TYPE_CONST_P(PgInterface, result); } +/**********************************************************************************************************************************/ +bool +pgDbIsTemplate(const String *const name) +{ + FUNCTION_TEST_BEGIN(); + FUNCTION_TEST_PARAM(STRING, name); + FUNCTION_TEST_END(); + + FUNCTION_TEST_RETURN(BOOL, strEqZ(name, "template0") || strEqZ(name, "template1")); +} + +/**********************************************************************************************************************************/ +bool +pgDbIsSystem(const String *const name) +{ + FUNCTION_TEST_BEGIN(); + FUNCTION_TEST_PARAM(STRING, name); + FUNCTION_TEST_END(); + + FUNCTION_TEST_RETURN(BOOL, strEqZ(name, "postgres") || pgDbIsTemplate(name)); +} + +/**********************************************************************************************************************************/ +bool +pgDbIsSystemId(const unsigned int id) +{ + FUNCTION_TEST_BEGIN(); + FUNCTION_TEST_PARAM(UINT, id); + FUNCTION_TEST_END(); + + FUNCTION_TEST_RETURN(BOOL, id < FirstNormalObjectId); +} + /*********************************************************************************************************************************** Check expected WAL segment size for older PostgreSQL versions ***********************************************************************************************************************************/ diff --git a/src/postgres/interface.h b/src/postgres/interface.h index 514f2784c..bbd8fe82c 100644 --- a/src/postgres/interface.h +++ b/src/postgres/interface.h @@ -67,13 +67,6 @@ Page size can only be changed at compile time and is not known to be well-tested ***********************************************************************************************************************************/ #define PG_PAGE_SIZE_DEFAULT ((unsigned int)(8 * 1024)) -/*********************************************************************************************************************************** -Define the minimum oid that can be used for a user object - -Everything below this number should have been created at initdb time. -***********************************************************************************************************************************/ -#define PG_USER_OBJECT_MIN_ID 16384 - /*********************************************************************************************************************************** Define default segment size and pages per segment @@ -127,6 +120,15 @@ typedef struct PgWal /*********************************************************************************************************************************** Functions ***********************************************************************************************************************************/ +// Is this a template database? +bool pgDbIsTemplate(const String *name); + +// Is this a system database, i.e. template or postgres? +bool pgDbIsSystem(const String *name); + +// Does this database have a system id, i.e. less than the minimum assignable user id? +bool pgDbIsSystemId(unsigned int id); + // Get info from pg_control PgControl pgControlFromFile(const Storage *storage); diff --git a/src/postgres/interface/static.vendor.h b/src/postgres/interface/static.vendor.h index a8d26dd99..a9208b81c 100644 --- a/src/postgres/interface/static.vendor.h +++ b/src/postgres/interface/static.vendor.h @@ -213,4 +213,50 @@ typedef PageHeaderData *PageHeader; */ #define PageIsNew(page) (((PageHeader) (page))->pd_upper == 0) +/*********************************************************************************************************************************** +Types from src/include/access/transam.h +***********************************************************************************************************************************/ + +// FirstNormalObjectId define +// --------------------------------------------------------------------------------------------------------------------------------- +/* + * Object ID (OID) zero is InvalidOid. + * + * OIDs 1-9999 are reserved for manual assignment (see .dat files in + * src/include/catalog/). Of these, 8000-9999 are reserved for + * development purposes (such as in-progress patches and forks); + * they should not appear in released versions. + * + * OIDs 10000-11999 are reserved for assignment by genbki.pl, for use + * when the .dat files in src/include/catalog/ do not specify an OID + * for a catalog entry that requires one. Note that genbki.pl assigns + * these OIDs independently in each catalog, so they're not guaranteed + * to be globally unique. Furthermore, the bootstrap backend and + * initdb's post-bootstrap processing can also assign OIDs in this range. + * The normal OID-generation logic takes care of any OID conflicts that + * might arise from that. + * + * OIDs 12000-16383 are reserved for unpinned objects created by initdb's + * post-bootstrap processing. initdb forces the OID generator up to + * 12000 as soon as it's made the pinned objects it's responsible for. + * + * OIDs beginning at 16384 are assigned from the OID generator + * during normal multiuser operation. (We force the generator up to + * 16384 as soon as we are in normal operation.) + * + * The choices of 8000, 10000 and 12000 are completely arbitrary, and can be + * moved if we run low on OIDs in any category. Changing the macros below, + * and updating relevant documentation (see bki.sgml and RELEASE_CHANGES), + * should be sufficient to do this. Moving the 16384 boundary between + * initdb-assigned OIDs and user-defined objects would be substantially + * more painful, however, since some user-defined OIDs will appear in + * on-disk data; such a change would probably break pg_upgrade. + * + * NOTE: if the OID generator wraps around, we skip over OIDs 0-16383 + * and resume with 16384. This minimizes the odds of OID conflict, by not + * reassigning OIDs that might have been assigned during initdb. Critically, + * it also ensures that no user-created object will be considered pinned. + */ +#define FirstNormalObjectId 16384 + #endif diff --git a/test/define.yaml b/test/define.yaml index 2bdb1c1ef..59a76a5ab 100644 --- a/test/define.yaml +++ b/test/define.yaml @@ -580,7 +580,7 @@ unit: # ---------------------------------------------------------------------------------------------------------------------------- - name: interface - total: 9 + total: 10 harness: postgres coverage: diff --git a/test/src/common/harnessPq.h b/test/src/common/harnessPq.h index 844634e5a..26de082ce 100644 --- a/test/src/common/harnessPq.h +++ b/test/src/common/harnessPq.h @@ -378,7 +378,9 @@ Macros for defining groups of functions that implement various queries and comma #define HRNPQ_MACRO_DATABASE_LIST_1(sessionParam, databaseNameParam) \ {.session = sessionParam, \ .function = HRNPQ_SENDQUERY, \ - .param = "[\"select oid::oid, datname::text, datlastsysoid::oid from pg_catalog.pg_database\"]", \ + .param = \ + "[\"select oid::oid, datname::text, (select oid::oid from pg_catalog.pg_database where datname = 'template0')" \ + " as datlastsysoid from pg_catalog.pg_database\"]", \ .resultInt = 1}, \ {.session = sessionParam, .function = HRNPQ_CONSUMEINPUT}, \ {.session = sessionParam, .function = HRNPQ_ISBUSY}, \ diff --git a/test/src/module/command/infoTest.c b/test/src/module/command/infoTest.c index 717e2cb6b..bd988b3ce 100644 --- a/test/src/module/command/infoTest.c +++ b/test/src/module/command/infoTest.c @@ -740,10 +740,10 @@ testRun(void) #define TEST_MANIFEST_DB \ "\n" \ "[db]\n" \ - "mail={\"db-id\":16456,\"db-last-system-id\":12168}\n" \ - "postgres={\"db-id\":12173,\"db-last-system-id\":12168}\n" \ - "template0={\"db-id\":12168,\"db-last-system-id\":12168}\n" \ - "template1={\"db-id\":1,\"db-last-system-id\":12168}\n" \ + "mail={\"db-id\":16456,\"db-last-system-id\":99999}\n" \ + "postgres={\"db-id\":12173,\"db-last-system-id\":99999}\n" \ + "template0={\"db-id\":12168,\"db-last-system-id\":99999}\n" \ + "template1={\"db-id\":1,\"db-last-system-id\":99999}\n" \ #define TEST_MANIFEST_FILE \ "\n" \ @@ -1765,8 +1765,8 @@ testRun(void) #define TEST_MANIFEST_NO_DB \ "\n" \ "[db]\n" \ - "template0={\"db-id\":12168,\"db-last-system-id\":12168}\n" \ - "template1={\"db-id\":1,\"db-last-system-id\":12168}\n" \ + "template0={\"db-id\":12168,\"db-last-system-id\":99999}\n" \ + "template1={\"db-id\":1,\"db-last-system-id\":99999}\n" \ #define TEST_MANIFEST_FILE_NO_CHECKSUM_ERROR \ "\n" \ diff --git a/test/src/module/command/restoreTest.c b/test/src/module/command/restoreTest.c index bcb2b687e..e6c8f7e87 100644 --- a/test/src/module/command/restoreTest.c +++ b/test/src/module/command/restoreTest.c @@ -1316,11 +1316,11 @@ testRun(void) MEM_CONTEXT_BEGIN(manifest->pub.memContext) { // Give non-systemId to postgres db - manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("postgres"), .id = 16385, .lastSystemId = 12168}); - manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("template0"), .id = 12168, .lastSystemId = 12168}); - manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("template1"), .id = 1, .lastSystemId = 12168}); - manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("user-made-system-db"), .id = 16380, .lastSystemId = 12168}); - manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF(UTF8_DB_NAME), .id = 16384, .lastSystemId = 12168}); + manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("postgres"), .id = 16385, .lastSystemId = 99999}); + manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("template0"), .id = 12168, .lastSystemId = 99999}); + manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("template1"), .id = 1, .lastSystemId = 99999}); + manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("user-made-system-db"), .id = 16380, .lastSystemId = 99999}); + manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF(UTF8_DB_NAME), .id = 16384, .lastSystemId = 99999}); manifestFileAdd( manifest, &(ManifestFile){.name = STRDEF(MANIFEST_TARGET_PGDATA "/" PG_PATH_BASE "/1/" PG_FILE_PGVERSION)}); manifestFileAdd( @@ -1420,7 +1420,7 @@ testRun(void) MEM_CONTEXT_BEGIN(manifest->pub.memContext) { - manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("test2"), .id = 32768, .lastSystemId = 12168}); + manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("test2"), .id = 32768, .lastSystemId = 99999}); manifestFileAdd( manifest, &(ManifestFile){.name = STRDEF(MANIFEST_TARGET_PGDATA "/" PG_PATH_BASE "/32768/" PG_FILE_PGVERSION)}); } @@ -1466,7 +1466,7 @@ testRun(void) MEM_CONTEXT_BEGIN(manifest->pub.memContext) { - manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("test3"), .id = 65536, .lastSystemId = 12168}); + manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("test3"), .id = 65536, .lastSystemId = 99999}); manifestFileAdd( manifest, &(ManifestFile){ .name = STRDEF(MANIFEST_TARGET_PGTBLSPC "/16387/PG_9.4_201409291/65536/" PG_FILE_PGVERSION)}); @@ -2601,7 +2601,7 @@ testRun(void) HRN_STORAGE_PUT_Z(storageRepoWrite(), STORAGE_REPO_BACKUP "/" TEST_LABEL_INCR "/bundle/2", "aXb"); // system db name - manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("template1"), .id = 1, .lastSystemId = 12168}); + manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("template1"), .id = 1, .lastSystemId = 99999}); // base/16384 directory manifestPathAdd( @@ -2632,7 +2632,7 @@ testRun(void) HRN_STORAGE_PUT(storageRepoWrite(), TEST_REPO_PATH "base/16384/16385", fileBuffer); // base/32768 directory - manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("test2"), .id = 32768, .lastSystemId = 12168}); + manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("test2"), .id = 32768, .lastSystemId = 99999}); manifestPathAdd( manifest, &(ManifestPath){ diff --git a/test/src/module/info/manifestTest.c b/test/src/module/info/manifestTest.c index adf58f771..90ca5ee77 100644 --- a/test/src/module/info/manifestTest.c +++ b/test/src/module/info/manifestTest.c @@ -1476,14 +1476,14 @@ testRun(void) #define TEST_MANIFEST_DB \ "\n" \ "[db]\n" \ - " mail\t={\"db-id\":16456,\"db-last-system-id\":12168}\n" \ - "#={\"db-id\":16453,\"db-last-system-id\":12168}\n" \ - "=={\"db-id\":16455,\"db-last-system-id\":12168}\n" \ - "[={\"db-id\":16454,\"db-last-system-id\":12168}\n" \ - "postgres={\"db-id\":12173,\"db-last-system-id\":12168}\n" \ - "template0={\"db-id\":12168,\"db-last-system-id\":12168}\n" \ - "template1={\"db-id\":1,\"db-last-system-id\":12168}\n" \ - SHRUG_EMOJI "={\"db-id\":18000,\"db-last-system-id\":12168}\n" + " mail\t={\"db-id\":16456,\"db-last-system-id\":99999}\n" \ + "#={\"db-id\":16453,\"db-last-system-id\":99999}\n" \ + "=={\"db-id\":16455,\"db-last-system-id\":99999}\n" \ + "[={\"db-id\":16454,\"db-last-system-id\":99999}\n" \ + "postgres={\"db-id\":12173,\"db-last-system-id\":99999}\n" \ + "template0={\"db-id\":12168,\"db-last-system-id\":99999}\n" \ + "template1={\"db-id\":1,\"db-last-system-id\":99999}\n" \ + SHRUG_EMOJI "={\"db-id\":18000,\"db-last-system-id\":99999}\n" #define TEST_MANIFEST_FILE \ "\n" \ @@ -1576,11 +1576,11 @@ testRun(void) TEST_MANIFEST_TARGET "\n" "[db]\n" - " mail\t={\"db-id\":16456,\"db-last-system-id\":12168}\n" - "#={\"db-id\":16453,\"db-last-system-id\":12168}\n" - "=={\"db-id\":16455,\"db-last-system-id\":12168}\n" - "[={\"db-id\":16454,\"db-last-system-id\":12168}\n" - "postgres={\"db-id\":12173,\"db-last-system-id\":12168}\n" + " mail\t={\"db-id\":16456,\"db-last-system-id\":99999}\n" + "#={\"db-id\":16453,\"db-last-system-id\":99999}\n" + "=={\"db-id\":16455,\"db-last-system-id\":99999}\n" + "[={\"db-id\":16454,\"db-last-system-id\":99999}\n" + "postgres={\"db-id\":12173,\"db-last-system-id\":99999}\n" TEST_MANIFEST_FILE TEST_MANIFEST_FILE_DEFAULT TEST_MANIFEST_LINK @@ -1631,19 +1631,19 @@ testRun(void) pckWriteArrayBeginP(dbList); pckWriteU32P(dbList, 12168); pckWriteStrP(dbList, STRDEF("template0")); - pckWriteU32P(dbList, 12168); + pckWriteU32P(dbList, 99999); pckWriteArrayEndP(dbList); pckWriteArrayBeginP(dbList); pckWriteU32P(dbList, 1); pckWriteStrP(dbList, STRDEF("template1")); - pckWriteU32P(dbList, 12168); + pckWriteU32P(dbList, 99999); pckWriteArrayEndP(dbList); pckWriteArrayBeginP(dbList); pckWriteU32P(dbList, 18000); pckWriteStrP(dbList, STRDEF(SHRUG_EMOJI)); - pckWriteU32P(dbList, 12168); + pckWriteU32P(dbList, 99999); pckWriteArrayEndP(dbList); pckWriteEndP(dbList); diff --git a/test/src/module/postgres/interfaceTest.c b/test/src/module/postgres/interfaceTest.c index f8ee464ef..162f25717 100644 --- a/test/src/module/postgres/interfaceTest.c +++ b/test/src/module/postgres/interfaceTest.c @@ -31,6 +31,21 @@ testRun(void) TEST_RESULT_STR_Z(pgVersionToStr(93456), "9.34", "infoPgVersionToString 93456"); } + // ***************************************************************************************************************************** + if (testBegin("pgDbIs*()")) + { + TEST_RESULT_BOOL(pgDbIsTemplate(STRDEF("template0")), true, "template0 is template"); + TEST_RESULT_BOOL(pgDbIsTemplate(STRDEF("template1")), true, "template1 is template"); + TEST_RESULT_BOOL(pgDbIsTemplate(STRDEF("postgres")), false, "postgres is not template"); + + TEST_RESULT_BOOL(pgDbIsSystem(STRDEF("postgres")), true, "postgres is system"); + TEST_RESULT_BOOL(pgDbIsSystem(STRDEF("template0")), true, "template0 is system"); + TEST_RESULT_BOOL(pgDbIsSystem(STRDEF("app")), false, "app is not system"); + + TEST_RESULT_BOOL(pgDbIsSystemId(16383), true, "16383 is system"); + TEST_RESULT_BOOL(pgDbIsSystemId(16384), false, "16384 is not system"); + } + // ***************************************************************************************************************************** if (testBegin("pgControlVersion()")) {