From: Nathan Bossart <nathandbossart@gmail.com>
To: Gurjeet Singh <gurjeet@singh.im>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Richard Veselý <richard.vesely@softea.sk>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: Fwd: BUG #18016: REINDEX TABLE failure
Date: Thu, 27 Jul 2023 16:14:41 -0700
Message-ID: <20230727231441.GB3612597@nathanxps13> (raw)
In-Reply-To: <CABwTF4XHEfL9rc25z+w9o2_Nmo+2_TAVm+LwB1tdtzu1+w12gA@mail.gmail.com>
References: <ZKirERBzkM48E0op@paquier.xyz>
<1457241.1688775601@sss.pgh.pa.us>
<CABwTF4XCTA=guV=4v3xkQnKhtqKsW2njeKeZCNtDhSFoYRWfDQ@mail.gmail.com>
<ZKpn+44jciv6Ym3/@paquier.xyz>
<1867443.1688912310@sss.pgh.pa.us>
<CABwTF4WX=m5pQvKXvLFJoEH=hSd6O=iZSqxVqHKjFm+iL-AO=w@mail.gmail.com>
<20230726175000.GA3259285@nathanxps13>
<CABwTF4U+t-8sD2y2xhgHyj=ovPMbrsNJO5F49nvZZtpwFkwWDA@mail.gmail.com>
<CABwTF4XyGzeEetn5AhsVTfY7zzBBcfjuMCfTLinOBWdbHbCWPw@mail.gmail.com>
<CABwTF4XHEfL9rc25z+w9o2_Nmo+2_TAVm+LwB1tdtzu1+w12gA@mail.gmail.com>
On Wed, Jul 26, 2023 at 06:43:18PM -0700, Gurjeet Singh wrote:
> On Wed, Jul 26, 2023 at 2:53 PM Gurjeet Singh <gurjeet@singh.im> wrote:
>> On Wed, Jul 26, 2023 at 10:50 AM Nathan Bossart
>> <nathandbossart@gmail.com> wrote:
>> > On Mon, Jul 10, 2023 at 09:35:05AM -0700, Gurjeet Singh wrote:
>> > + * This should be done after the suppression of the use of indexes (above),
>> > + * because the recursive call to reindex_relation() below will invoke
>> > + * CommandCounterIncrement(), which may prevent enumeration of the indexes
>> > + * on the table.
>> >
>> > I'm not following this. We've already obtained the list of index OIDs
>> > before this point. Does this create problems when we try to open and lock
>> > the relations? And if so, how?
>>
>> This comment is calling out the fact that there's a recursive call to
>> reindex_relation() inside the 'if' code block, and that that
>> reindex_relation() calls CCI. Hence this 'if' code block should _not_
>> be placed before the the calls to RelationGetIndexList() and
>> SetReindexPending(). Because if we do, then the CCI done by
>> reindex_relation() will impact what RelationGetIndexList() sees.
>>
>> This is to match the expectation set for the
>> REINDEX_REL_SUPPRESS_INDEX_USE flag.
>
> Given that the issue is already explained by the flag's comments above
> the function, this comment paragraph in the patch may be considered
> extraneous. Feel free to remove it, if you think so.
>
> I felt the need for that paragraph, because it doesn't feel obvious to
> me as to why we can't simply reindex the toast table as the first
> thing in this function; the toast table reindex will trigger CCI, and
> that'd be bad if done before RelationGetIndexList().
I see. I'd suggest referencing the comment above the function, but in
general I do think having a comment about this is appropriate.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
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: pgsql-bugs@postgresql.org
Cc: nathandbossart@gmail.com, gurjeet@singh.im, tgl@sss.pgh.pa.us, michael@paquier.xyz, richard.vesely@softea.sk, pgsql-bugs@lists.postgresql.org
Subject: Re: Fwd: BUG #18016: REINDEX TABLE failure
In-Reply-To: <20230727231441.GB3612597@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox