Sorry, the previous revision did not update regression test part
towards the latest one.
2011/6/19 Kohei KaiGai <kaigai@kaigai.gr.jp>:
> Thanks for your review.
>
> 2011/6/19 Robert Haas <robertmhaas@gmail.com>:
>> On Tue, Jun 14, 2011 at 8:06 AM, Kohei KaiGai <kaigai@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 <kaigai@kaigai.gr.jp>
>
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
