On Mon, Dec 5, 2011 at 2:09 AM, Noah Misch <noah@leadboat.com> wrote:
> Your committed patch looks great overall. A few cosmetic points:
Thanks for the review.
> That last sentence needs a word around "might things".
Fixed.
>> AcceptInvalidationMessages();
>
> The above call can go away, now.
Doesn't that still protect us against namespace-shadowing issues?
RangeVarGetRelid doesn't actually AcceptInvalidationMessages() at the
top.
> That sentence needs a word around "so need".
Fixed.
Attached please find a patch with some more fixes on this same general
theme. This one tackles renaming of relations, columns, and triggers;
and changing the schema of relations. In these cases, the current
code does a permissions check before locking the table (which is good)
and uses RangeVarGetRelid() to guard against "cache lookup failure"
errors caused by concurrent DDL (also good). However, if the referent
of the name changes during the lock wait, we don't recheck
permissions; we just allow the rename or schema change on the basis
that the user had permission to do it to the relation that formerly
had that name. While this is pretty minor as security concerns go, it
seems best to clean it up, so this patch does that.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
