Re: patch: ALTER TABLE IF EXISTS

Hello

2012/1/3 Robert Haas <robertmhaas@gmail.com>:
> On Tue, Jan 3, 2012 at 10:38 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> Hello
>>
>> 2012/1/3 Robert Haas <robertmhaas@gmail.com>:
>>> On Mon, Jan 2, 2012 at 12:01 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>>>> here is updated patch
>>>
>>> I think the comments in parse_utilcmd.c probably need a bit of adjustment.
>>
>> I don't see it - there is only one comment and it is adjusted with
>> "if" statement.
>>
>> please, show it
>
> Well, basically, the comment preceding the part you altered say "the
> lock level requested here", but "here" is getting spread out quite a
> bit more with this code change.  Maybe that doesn't matter.
>
> However, on further examination, this is a pretty awkward way to write
> the code.  Why not something like this:
>
> rel = relation_openrv_extended(stmt->relation, lockmode, stmt->missing_ok);
> if (rel == NULL)
> {
>    ereport(...);
>    return NIL;
> }
>
> Maybe the intent of sticking heap_openrv_extended() into the upper
> branch of the if statement is to try to bounce relations that aren't
> tables, but that's not actually what that function does (e.g. a
> foreign table will slip through).  And I think if we're going to have
> IF EXISTS support for ALTER TABLE at all, we ought to have it for the
> whole family of related statements: ALTER VIEW, ALTER SEQUENCE, ALTER
> FOREIGN TABLE, etc., not just ALTER TABLE itself.
>

jup, we can continue in enhancing step by step.

I change a patch and now ALTER TABLE, ALTER INDEX, ALTER SEQUENCE and
ALTER VIEW has IF EXISTS clause

Regards

Pavel


> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company