On 2021-Jan-12, Álvaro Herrera wrote:
> On 2021-Jan-12, Álvaro Herrera wrote:
>
> > > For the 0001 patch, since ReindexIndexInfo is used only within
> > > ReindexRelationConcurrently() I think it’s a function-local structure
> > > type. So we can declare it within the function. What do you think?
> >
> > That's a good idea. Pushed with that change, thanks.
>
> Here's the other patch, with comments fixed per reviews, and rebased to
> account for the scope change.
Pushed. At the last minute I decided to back off on reverting the flag
set that DefineIndex has, because there was no point in doing that. I
also updated the comment in (two places of) ReindexRelationConcurrently
about why we don't do it there. The previous submission had this:
> + /*
> + * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
> + * it only acquires an Xid to do some catalog manipulations, after the
> + * wait is over.
> + */
but this fails to point out that the main reason not to do it, is to
avoid having to decide whether to do it or not when some indexes are
safe and others aren't. So I changed to:
+ /*
+ * While we could set PROC_IN_SAFE_IC if all indexes qualified, there's no
+ * real need for that, because we only acquire an Xid after the wait is
+ * done, and that lasts for a very short period.
+ */
Thanks!
--
Álvaro Herrera Valdivia, Chile
"The eagle never lost so much time, as
when he submitted to learn of the crow." (William Blake)