Re: [PATCH] Small refactoring of inval.c and inval.h - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: [PATCH] Small refactoring of inval.c and inval.h
Date
Msg-id ZUia1gPPMtx26oMy@paquier.xyz
Whole thread Raw
In response to Re: [PATCH] Small refactoring of inval.c and inval.h  (Aleksander Alekseev <aleksander@timescale.com>)
Responses Re: [PATCH] Small refactoring of inval.c and inval.h
List pgsql-hackers
On Tue, Jul 25, 2023 at 06:38:46PM +0300, Aleksander Alekseev wrote:
> Unless of course we want to change its signature too. I don't think
> this is going to be a good API change.

 extern void InvalidateSystemCaches(void);
-extern void InvalidateSystemCachesExtended(bool debug_discard);

Indeed, that looks a bit strange, but is there a strong need in
removing it, as you are proposing?  There is always a risk that this
could be called by some out-of-core code.  This is one of these
things where we could break something just for the sake of breaking
it, so there is no real benefit IMO.

>> 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.
>
> Fixed.

This part looks OK to me, so I'll see about applying this part of the
patch.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Richard Guo
Date:
Subject: Re: Compiling warnings on old GCC
Next
From: Shlok Kyal
Date:
Subject: Re: [ psql - review request ] review request for \d+ tablename, \d+ indexname indenting