From: Tom Lane <tgl@sss.pgh.pa.us>
To: "David G. Johnston" <david.g.johnston@gmail.com>
cc: =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@alvh.no-ip.org>,
        Sebastien Flaesch <sebastien.flaesch@4js.com>,
        Adrian Klaver <adrian.klaver@aklaver.com>,
        M Tarkeshwar Rao <m.tarkeshwar.rao@ericsson.com>,
        "pgsql-general@postgresql.org" <pgsql-general@postgresql.org>
Subject: Re: After upgrading libpq, the same function(PQftype) call returns a different OID
In-reply-to: <CAKFQuwZYCktj5Ect4f5Mx1OejCRbQ+MoKWQN+F=2Oid5M=3E7Q@mail.gmail.com>
References: <202503201748.wxkazqupyvuk@alvherre.pgsql> <1092933.1742496844@sss.pgh.pa.us> <CAKFQuwZYCktj5Ect4f5Mx1OejCRbQ+MoKWQN+F=2Oid5M=3E7Q@mail.gmail.com>
Comments: In-reply-to "David G. Johnston" <david.g.johnston@gmail.com>
	message dated "Thu, 20 Mar 2025 13:08:54 -0700"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <1134400.1742507717.0@sss.pgh.pa.us>
Content-Transfer-Encoding: 8bit
Date: Thu, 20 Mar 2025 17:56:05 -0400
Message-ID: <1134562.1742507765@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/1134562.1742507765%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <1134400.1742507717.1@sss.pgh.pa.us>
Content-Transfer-Encoding: 8bit

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Thu, Mar 20, 2025 at 11:54 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think it's a mistake to suppose that pg_type_d.h is the only
>> place where there's a risk of confusion.  We should be thinking
>> about this more generally: genbki.pl is taking zero thought to
>> make what it emits readable.  I think it would help to
>> label the sections it emits, perhaps along the lines of
>> /* Auto-generated OID macros */

> I'd consider this enough for the moment, so long as we explicitly address
> the cross-version constancy of the OID values associated with each type.

That's documented elsewhere, I believe.  For the foo_d.h files,
I think it'd be sufficient to do something like 0001 attached.

Also, while checking out the results, I noticed that pg_class.h
has an "extern" in the wrong place: it's exposed to client code
which can have no use for it.  That extern doesn't mention any
backend-only typedefs, so it's fairly harmless, but it's still
a clear example of somebody not reading the memo.  Hence 0002.

> I'm not going to dive deep enough to make more targeted suggestions.  It
> does seem, though, that "client code" would seem mostly interested in these
> OIDs and not stuff like the attribute numbers of the columns in pg_type.  I
> get a distinct feel of one file serving multiple use cases.

Well, yes it does, but that doesn't make those symbols useless to
client code.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="0001-label-sections-of-_d.h-files.patch";
	charset="us-ascii"
Content-ID: <1134400.1742507717.2@sss.pgh.pa.us>
Content-Description: 0001-label-sections-of-_d.h-files.patch

diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 2501307c92e..df3231fcd41 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -480,6 +480,8 @@ foreach my $catname (@catnames)
 
 EOM
 
+	printf $def "/* Macros related to the structure of $catname */\n\n";
+
 	# Emit OID macros for catalog's OID and rowtype OID, if wanted
 	printf $def "#define %s %s\n",
 	  $catalog->{relation_oid_macro}, $catalog->{relation_oid}
@@ -561,6 +563,7 @@ EOM
 	print $def "\n#define Natts_$catname $attnum\n\n";
 
 	# Emit client code copied from source header
+	printf $def "/* Definitions copied from ${catname}.h */\n\n";
 	foreach my $line (@{ $catalog->{client_code} })
 	{
 		print $def $line;
@@ -573,6 +576,9 @@ EOM
 		print $bki "open $catname\n";
 	}
 
+	printf $def
+	  "\n/* OID symbols for objects defined in ${catname}.dat */\n\n";
+
 	# For pg_attribute.h, we generate data entries ourselves.
 	if ($catname eq 'pg_attribute')
 	{

------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="0002-place-extern-decl-correctly.patch";
	charset="us-ascii"
Content-ID: <1134400.1742507717.3@sss.pgh.pa.us>
Content-Description: 0002-place-extern-decl-correctly.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class=
.h
index fa96ba07bf4..07d182da796 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -231,8 +231,8 @@ MAKE_SYSCACHE(RELNAMENSP, pg_class_relname_nsp_index, =
128);
 	 (relkind) =3D=3D RELKIND_TOASTVALUE || \
 	 (relkind) =3D=3D RELKIND_MATVIEW)
 =

-extern int	errdetail_relkind_not_supported(char relkind);
-
 #endif							/* EXPOSE_TO_CLIENT_CODE */
 =

+extern int	errdetail_relkind_not_supported(char relkind);
+
 #endif							/* PG_CLASS_H */

------- =_aaaaaaaaaa0--