Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce)
От | Sadhuprasad Patro |
---|---|
Тема | Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce) |
Дата | |
Msg-id | CAFF0-CEPkpnc+SbPxqm0Q6A6cKjmNN+PndYf21+T+gQtiM0BGg@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce) (Shruthi Gowda <gowdashru@gmail.com>) |
Ответы |
Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce)
(Robert Haas <robertmhaas@gmail.com>)
Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce) (Shruthi Gowda <gowdashru@gmail.com>) |
Список | pgsql-hackers |
On Tue, Oct 26, 2021 at 6:55 PM Shruthi Gowda <gowdashru@gmail.com> wrote: > > > I have revised the patch w.r.t the way 'create_storage' is interpreted > in heap_create() along with some minor changes to preserve the DBOID > patch. > Hi Shruthi, I am reviewing the attached patches and providing a few comments here below for patch "v5-0002-Preserve-database-OIDs-in-pg_upgrade.patch" 1. --- a/doc/src/sgml/ref/create_database.sgml +++ b/doc/src/sgml/ref/create_database.sgml @@ -31,7 +31,8 @@ CREATE DATABASE <replaceable class="parameter">name</replaceable> - [ IS_TEMPLATE [=] <replaceable class="parameter">istemplate</replaceable> ] ] + [ IS_TEMPLATE [=] <replaceable class="parameter">istemplate</replaceable> ] + [ OID [=] <replaceable class="parameter">db_oid</replaceable> ] ] Replace "db_oid" with 'oid'. Below in the listitem, we have mentioned 'oid'. 2. --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c + if ((dboid < FirstNormalObjectId) && + (strcmp(dbname, "template0") != 0) && + (!IsBinaryUpgrade)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)), + errmsg("Invalid value for option \"%s\"", defel->defname), + errhint("The specified OID %u is less than the minimum OID for user objects %u.", + dboid, FirstNormalObjectId)); + } Are we sure that 'IsBinaryUpgrade' will be set properly, before the createdb function is called? Can we recheck once ? 3. @@ -504,11 +525,15 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) */ pg_database_rel = table_open(DatabaseRelationId, RowExclusiveLock); - do + /* Select an OID for the new database if is not explicitly configured. */ + if (!OidIsValid(dboid)) { - dboid = GetNewOidWithIndex(pg_database_rel, DatabaseOidIndexId, - Anum_pg_database_oid); - } while (check_db_file_conflict(dboid)); I think we need to do 'check_db_file_conflict' for the USER given OID also.. right? It may already be present. 4. --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c /* + * Create template0 database with oid Template0ObjectId i.e, 4 + */ + Better to mention here, why OID 4 is reserved for template0 database?. 5. + /* + * Create template0 database with oid Template0ObjectId i.e, 4 + */ + static const char *const template0_setup[] = { + "CREATE DATABASE template0 IS_TEMPLATE = true ALLOW_CONNECTIONS = false OID " + CppAsString2(Template0ObjectId) ";\n\n", Can we write something like, 'OID = CppAsString2(Template0ObjectId)'? mention "=". 6. + + /* + * We use the OID of postgres to determine datlastsysoid + */ + "UPDATE pg_database SET datlastsysoid = " + " (SELECT oid FROM pg_database " + " WHERE datname = 'postgres');\n\n", + Make the above comment a single line comment. 7. There are some spelling mistakes in the comments as below, please correct the same + /* + * Make sure that binary upgrade propogate the database OID to the new =====> correct spelling + * cluster + */ +/* OID 4 is reserved for Templete0 database */ ====> Correct spelling +#define Template0ObjectId 4 I am reviewing another patch "v5-0001-Preserve-relfilenode-and-tablespace-OID-in-pg_upg" as well and will provide the comments soon if any... Thanks & Regards SadhuPrasad EnterpriseDB: http://www.enterprisedb.com
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Mark DilgerДата:
Сообщение: Re: Optionally automatically disable logical replication subscriptions on error