On 2/5/21 8:43 AM, Michael Paquier wrote:
> On Fri, Feb 05, 2021 at 08:22:17AM +0100, Ronan Dunklau wrote:
>> I'm not surprised by this answer, the good news is it's being back-patched.
>
> Yes, I have no problem with that. Until this gets fixed, the damage
> can be limited with an extra ALTER INDEX, that takes a
> ShareUpdateExclusiveLock so there is no impact on the concurrent
> activity.
>
>> Looks good to me ! Thank you.
>
> Thanks for looking at it. Tomas, do you have any comments?
> --
Not really.
Copying this info in index_concurrently_swap seems a bit strange - we're
copying other stuff there, but this is modifying something we've already
copied before. I understand why we do it there to make this
backpatchable, but maybe it'd be good to mention this in a comment (or
at least the commit message). We could do this in the backbranches only
and the "correct" way in master, but that does not seem worth it.
One minor comment - the code says this:
/* no need for a refresh if both match */
if (attstattarget == att->attstattarget)
continue;
Isn't that just a different way to say "attstattarget is not default")?
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company