Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

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

In response to

Responses

Browse pgsql-hackers by date

  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