Re: IndexAmRoutine aminsertcleanup function can be NULL?

Japin Li <japinli@hotmail.com>

From: Japin Li <japinli@hotmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Peter Smith <smithpb2250@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-07-23T04:07:56Z
Lists: pgsql-hackers

Attachments

On Tue, 22 Jul 2025 at 14:36, Michael Paquier <michael@paquier.xyz> wrote:
> On Thu, Jul 17, 2025 at 01:34:42PM +0800, Japin Li wrote:
>> On Wed, 16 Jul 2025 at 10:08, Peter Smith <smithpb2250@gmail.com> wrote:
>>> What's going on there? Is it just an accidentally missing "/* can be
>>> NULL */" comment?
>> 
>> It appears commit c1ec02be1d79 is missing the comment.
>
> Agreed.  That's user-visible, so backpatched down to v17.
>
>> How about asserting the existence of all required callbacks, similar to
>> GetTableAmRoutine()?
>
> Hmm, we could do something like that to enforce a stronger policy,
> yes.  At least that seems sensible to me to be able to detect faster a
> problem rather than waiting for the backend to report the issue with a
> crash when testing a dedicated code path where one of these callbacks
> is run.  As a HEAD-only thing, of course.
>
> I was reading the page, and some of the functions in the docs are
> documented as optional with the matching NULL description.  It is 
> the case of aminsertcleanup as well, one paragraph above with the
> part about "may define".

PFA to assert all required IndexAM callbacks are present.

-- 
Regards,
Japin Li

Commits

  1. Add assertions for all the required index AM callbacks

  2. doc: Inform about aminsertcleanup optional NULLness

  3. Reuse BrinDesc and BrinRevmap in brininsert