Re: Consistently use the XLogRecPtrIsInvalid() macro
Álvaro Herrera <alvherre@kurilemu.de>
From: Álvaro Herrera <alvherre@kurilemu.de>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Peter Eisentraut <peter@eisentraut.org>, Michael Paquier <michael@paquier.xyz>, Heikki Linnakangas <hlinnaka@iki.fi>, Quan Zongliang <quanzongliang@yeah.net>, pgsql-hackers@lists.postgresql.org
Date: 2025-11-18T17:57: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 →
-
Replace literal 0 with InvalidXLogRecPtr for XLogRecPtr assignments
- ec3174407164 19 (unreleased) landed
-
Replace pointer comparisons and assignments to literal zero with NULL
- ec782f56b0c3 19 (unreleased) landed
-
Use XLogRecPtrIsValid() in various places
- a2b02293bc65 19 (unreleased) landed
-
Introduce XLogRecPtrIsValid()
- d2965f627fe3 14.20 landed
- c0031d461324 18.1 landed
- 723cc84db50a 16.11 landed
- 49b45999f3b2 15.15 landed
- 33727aff18d0 17.7 landed
- 20bafb097288 13.23 landed
- 06edbed47862 19 (unreleased) landed
On 2025-Nov-18, Bertrand Drouvot wrote:
> Hi,
>
> On Tue, Nov 18, 2025 at 04:54:32PM +0100, Peter Eisentraut wrote:
> > RegProcedureIsValid() doesn't add any value over OidIsValid, and we
> > don't have any RegXXXIsValid() for any of the other regxxx types.
> > So if we were to do anything about this, I would just remove it.
>
> The patch makes use of it because it exists. I agree that we could
> also remove it (for the reasons you mentioned above), I'll do that.
RegProcedure actually predates all of our reg* types -- it goes back to
the initial commit of Postgres in 1989, according to
https://github.com/kelvich/postgres_pre95/commit/c4af0cb9d2a391815eb513416d95d49760202a42#diff-843ff64714a2c04906cdc890ccf6aedd0444e395e4ec412e70465638e80b2011R182
+/*
+ * RegProcedureIsValid
+ */
+bool
+RegProcedureIsValid(procedure)
+ RegProcedure procedure;
+{
+ return (ObjectIdIsValid(procedure));
+}
I'm not sure what to think now about this whole idea of removing it.
Maybe there's some documentation value in it -- that line is saying,
this is not just any Oid, it's the Oid of a procedure. Is this
completely useless?
> > For OidIsValid etc., I don't think this improves the notation. It
> > is well understood that InvalidOid is 0.
>
> I agree and that's perfectly valid to use 0 (unless InvalidOid does not mean
> 0 anymore in the future for whatever reasons). That said I think that using
> the macro makes the code more consistent (same spirit as a2b02293bc6).
Well, what we were trying to do there was get rid of
XLogRecPtrIsInvalid(), which was clearly going against the grain re.
other IsValid macros. We got rid of the comparisons with 0 and
InvalidXLogRecPtr as a secondary effect, but that wasn't the main point.
This new patch series is much noisier and it's not at all clear to me
that we're achieving anything very useful. In fact, looking at proposed
changes, I would argue that there are several places that end up worse.
Honestly I'd leave this alone.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Ellos andaban todos desnudos como su madre los parió, y también las mujeres,
aunque no vi más que una, harto moza, y todos los que yo vi eran todos
mancebos, que ninguno vi de edad de más de XXX años" (Cristóbal Colón)