Re: [PATCH] Small refactoring of inval.c and inval.h

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)