From: | Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | PgHacker <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address |
Date: | 2011-06-19 11:40:28 |
Message-ID: | BANLkTimdMHYCTu31pdgTqOJdEMcK0KBnLw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Sorry, the previous revision did not update regression test part
towards the latest one.
2011/6/19 Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>:
> Thanks for your review.
>
> 2011/6/19 Robert Haas <robertmhaas(at)gmail(dot)com>:
>> On Tue, Jun 14, 2011 at 8:06 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)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 <kaigai(at)kaigai(dot)gr(dot)jp>
>
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>
Attachment | Content-Type | Size |
---|---|---|
pgsql-v9.2-drop-reworks-part-0.3.patch | application/octet-stream | 21.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Florian Pflug | 2011-06-19 11:43:43 | Re: patch for 9.2: enhanced errors |
Previous Message | Pavel Stehule | 2011-06-19 11:29:49 | Re: plpgsql performance - SearchCatCache issue |