Re: ALTER TABLE lock strength reduction patch is unsafe

On 3 March 2014 16:36, Robert Haas <robertmhaas@gmail.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