public inbox for [email protected]  

help / color / mirror / Atom feed

From: Alvaro Herrera <[email protected]>
To: Aleksander Alekseev <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PATCH] Small refactoring of inval.c and inval.h
Date: Tue, 25 Jul 2023 16:35:09 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAJ7c6TMX2dd0g91UKvcC+CVygKQYJkKJq1+ZzT4rOK42+b53=w@mail.gmail.com>

On 2023-Jul-25, Aleksander Alekseev wrote:

> Hi,
> 
> The proposed patch is a small refactoring of inval.{c,h}:
> 
> """
>     The "public functions" separator comment doesn't reflect reality anymore.
>     We could rearrange the order of the functions. However, this would
> complicate
>     back-porting of the patches, thus removing the comment instead.
> 
>     InvalidateSystemCachesExtended() is an internal function of inval.c. Make it
>     static.
> """

Hmm, this *Extended function looks a bit funny, and I think it's because
it's part of a backpatched bugfix that didn't want to modify ABI.  If
we're modifying this code, maybe we should get rid of the shim, that is,
move the boolean argument to InvalidateSystemCaches() and get rid of the
*Extended() version altogether.

As for the /*--- public functions ---*/ comment, that one was just not
moved by b89e151054a0, which should have done so; but even at that
point, it had already been somewhat broken by the addition of
PrepareInvalidationState() (a static function) in 6cb4afff33ba below it.
If we really want to clean this up, we could move PrepareInvalidationState()
to just below RegisterSnapshotInvalidation() and move the "public
functions" header to just below it (so that it appears just before
InvalidateSystemCaches).

If we just remove the "public functions" comment, then we're still
inside a section that starts with the "private support functions"
comment, which seems even worse to me than the current situation.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Saca el libro que tu religión considere como el indicado para encontrar la
oración que traiga paz a tu alma. Luego rebootea el computador
y ve si funciona" (Carlos Duclós)

view thread (3+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Small refactoring of inval.c and inval.h
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox