Sorry, the previous revision did not update regression test part towards the latest one.
2011/6/19 Kohei KaiGai <kai...@kaigai.gr.jp>: > Thanks for your review. > > 2011/6/19 Robert Haas <robertmh...@gmail.com>: >> On Tue, Jun 14, 2011 at 8:06 AM, Kohei KaiGai <kai...@kaigai.gr.jp> wrote: >>> The attached patch is a preparation to rework implementation of DROP >>> statement >>> using a common code. That intends to apply get_object_address() to resolve >>> name >>> of objects to be removed, and eventually minimizes the number of places to >>> put >>> permission checks. >>> >>> Its first step is an enhancement of get_object_address; to accept >>> 'missing_ok' >>> argument to handle cases when IF EXISTS clause is supplied. >>> If 'missing_ok' was true and the supplied name was not found, the patched >>> get_object_address() returns an ObjectAddress with InvalidOid as objectId. >>> If 'missing_ok' was false, its behavior is not changed. >> >> Let's consistently make missing_ok the last argument to all of the >> functions to which we're adding it. >> > OK. I revised position of the 'missing_ok' argument. > >> I don't think it's a good idea for get_relation_by_qualified_name() to >> be second-guessing the error message that RangeVarGetRelid() feels >> like throwing. >> > OK. I revised the patch to provide 'true' on RangeVarGetRelid(). > Its side effect is on error messages when user gives undefined object name. > >> I think that attempting to fetch the column foo.bar when foo doesn't >> exist should be an error even if missing_ok is true. I believe that's >> consistent with what we do elsewhere. (If it *were* necessary to open >> the relation without failing if it's not there, you could use >> try_relation_openrv instead of doing as you've done here.) >> > It was fixed. AlterTable() already open the relation (without missing_ok) > in the case when we drop a column, so I don't think we need to acquire > relation locks if the supplied column was missing. > >> There is certainly a more compact way of writing the logic in >> get_object_address_typeobj. Also, I think that function should be >> called get_object_address_type(); the "obj" on the end seems >> redundant. >> > I renamed the function name to get_object_address_type(), and > consolidate initialization of ObjectAddress variables. > > Thanks, > -- > KaiGai Kohei <kai...@kaigai.gr.jp> > -- KaiGai Kohei <kai...@kaigai.gr.jp>
pgsql-v9.2-drop-reworks-part-0.3.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
