| From: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
|---|---|
| To: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
| Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Quan Zongliang <quanzongliang(at)yeah(dot)net>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Consistently use the XLogRecPtrIsInvalid() macro |
| Date: | 2025-11-18 17:57:33 |
| Message-ID: | 202511181738.yokxzijfdfx7@alvherre.pgsql |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2025-11-18 18:21:22 | Re: [PATCH] Allow complex data for GUC extra. |
| Previous Message | David Geier | 2025-11-18 17:54:16 | Re: Use merge-based matching for MCVs in eqjoinsel |