On Mon, Mar 3, 2014 at 11:29 AM, Simon Riggs <si...@2ndquadrant.com> wrote: > On 3 March 2014 16:06, Robert Haas <robertmh...@gmail.com> wrote: >> On Sun, Mar 2, 2014 at 4:50 AM, Simon Riggs <si...@2ndquadrant.com> wrote: >>> v20 includes slightly re-ordered checks in GetLockLevel, plus more >>> detailed comments on each group of subcommands. >>> >>> Also corrects grammar as noted by Vik. >>> >>> Plus adds an example of usage to the docs. >> >> This patch contains a one line change to >> src/bin/pg_dump/pg_backup_archiver.c which seems not to belong. >> >> 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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
