Re: ALTER TABLE lock strength reduction patch is unsafe

On 3 March 2014 16:36, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

>>> This hunk in ATRewriteCatalogs() looks scary:
>>>
>>> + /*
>>> + * If we think we might need to add/re-add toast tables then
>>> + * we currently need to hold an AccessExclusiveLock.
>>> + */
>>> + if (lockmode < AccessExclusiveLock)
>>> + return;
>>>
>>> It would make sense to me to add an Assert() or elog() check inside
>>> the subsequent loop to verify that the lock level is adequate ... but
>>> just returning silently seems like a bad idea.
>>
>> OK, I will check elog.
>>
>>> I have my doubts about whether it's safe to do AT_AddInherit,
>>> AT_DropInherit, AT_AddOf, or AT_DropOf with a full lock. All of those
>>> can change the tuple descriptor, and we discussed, back when we did
>>> this the first time, the fact that the executor may get *very* unhappy
>>> if the tuple descriptor changes in mid-execution. I strongly suspect
>>> these are unsafe with less than a full AccessExclusiveLock.
>>
>> I'm happy to change those if you feel there is insufficient evidence.
>
> Not sure what that means, but yes, I think the lock levels on those
> need to be increased.

v21 with all requested changes, comments and cleanup

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services