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. I don't personally feel that it would matter to usability to keep locks for those at AccessExclusiveLock, especially since they are otherwise fast. Some others might be kept higher also. I'm merely trying to balance between requests to reduce to minimal theoretical level and fears that anything less than AccessExclusiveLock is a problem. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
