Horton

Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce)

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: Shruthi Gowda <gowdashru@gmail.com>
Cc: Julien Rouhaud <rjuju123@gmail.com>, Sadhuprasad Patro <b.sadhu@gmail.com>, Stephen Frost <sfrost@snowman.net>, Bruce Momjian <bruce@momjian.us>, Andres Freund <andres@anarazel.de>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Masahiko Sawada <sawada.mshk@gmail.com>, Tom Kincaid <tomjohnkincaid@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, PostgreSQL Development <pgsql-hackers@postgresql.org>, Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2022-01-20T14:27:33Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Rethink method for assigning OIDs to the template0 and postgres DBs.

  2. pg_upgrade: Preserve database OIDs.

  3. pg_upgrade: Preserve relfilenodes and tablespace OIDs.

  4. Fix for new Boolean node

  5. Improve error handling of HMAC computations

  6. Add macro RelationIsPermanent() to report relation permanence

  7. Enhance nbtree index tuple deletion.

On Thu, Jan 20, 2022 at 7:09 AM Shruthi Gowda <gowdashru@gmail.com> wrote:
> > Here's an updated version in which I've reverted the changes to gram.y
> > and tried to improve the comments and documentation. Could you have a
> > look at implementing (2) above?
>
> Attached is the patch that implements comment (2).

This probably needs minor rebasing on account of the fact that I just
pushed the patch to remove datlastsysoid. I intended to do that before
you posted a new version to save you the trouble, but I was too slow
(or you were too fast, however you want to look at it).

+ errmsg("Invalid value for option \"%s\"", defel->defname),

Per the "error message style" section of the documentation, primary
error messages neither begin with a capital letter nor end with a
period, while errdetail() messages are complete sentences and thus
both begin with a capital letter and end with a period. But what I
think you should really do here is get rid of the error detail and
convey all the information in a primary error message. e.g. "OID %u is
a system OID", or maybe better, "OIDs less than %u are reserved for
system objects".

+ errmsg("database oid %u is already used by database %s",
+ errmsg("data directory exists for database oid %u", dboid));

Usually we write "OID" rather than "oid" in error messages. I think
maybe it would be best to change the text slightly too. I suggest:

database OID %u is already in use by database \"%s\"
data directory already exists for database with OID %u

+ * it would fail. To avoid that, assign a fixed OID to template0 and
+ * postgres rather than letting the server choose one.

a fixed OID -> fixed OIDs
one -> them

Or maybe put this comment back the way I had it and just talk about
postgres, and then change the comment in make_postgres to say "Assign
a fixed OID to postgres, for the same reasons as template0."

+ /*
+ * Make sure that binary upgrade propagate the database OID to the new
+ * cluster
+ */

This comment doesn't really seem necessary. It's sort of self-explanatory.

+# Push the OID that is reserved for template0 database.
+my $Template0ObjectId =
+  Catalog::FindDefinedSymbol('access/transam.h', '..', 'Template0ObjectId');
+push @{$oids}, $Template0ObjectId;

Don't you need to do this for PostgresObjectId also?

-- 
Robert Haas
EDB: http://www.enterprisedb.com