Re: PATCH: Add REINDEX tag to event triggers - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: PATCH: Add REINDEX tag to event triggers
Date
Msg-id ZW5kf4WOaZOJMgls@paquier.xyz
Whole thread Raw
In response to Re: PATCH: Add REINDEX tag to event triggers  (jian he <jian.universality@gmail.com>)
Responses Re: PATCH: Add REINDEX tag to event triggers
List pgsql-hackers
On Tue, Dec 05, 2023 at 12:49:12AM +0800, jian he wrote:
> On Mon, Dec 4, 2023 at 11:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
>> Please look at the assertion failure triggered when REINDEX processed by an event trigger:
>> CREATE FUNCTION etf() RETURNS EVENT_TRIGGER AS $$ BEGIN PERFORM 1; END $$ LANGUAGE plpgsql;
>> CREATE EVENT TRIGGER et ON ddl_command_end EXECUTE FUNCTION etf();
>> CREATE TABLE t (i int);
>> REINDEX (CONCURRENTLY) TABLE t;
>
> In indexcmd.c, function, ReindexRelationConcurrently
> if (indexIds == NIL)
> {
> PopActiveSnapshot();
> return false;
> }
>
> So there is no snapshot left, then PERFORM 1; need a snapshot.

Popping a snapshot at this stage when there are no indexes has been a
decision taken by the original commit in 5dc92b844e68 because we had
no need for it yet, but we may do now depending on the function
triggered.  I have been looking at the whole stack and something like
the attached to make a pop conditional seems to be sufficient to
satisfy all the cases I've been able to come up with, including the
one reported here.

Alexander, do you see any hole in that?  Perhaps the snapshot push
should be more aggressive at the end of ReindexRelationConcurrently()
as well (in the last transaction when rebuilds happen)?
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Davin Shearer
Date:
Subject: Re: Emitting JSON to file using COPY TO
Next
From: zhihuifan1213@163.com
Date:
Subject: Re: Avoid detoast overhead when possible