Re: Overhaul of type attributes modification

From: Thom Brown <thom(at)linux(dot)com>
To: Guillaume Lelarge <guillaume(at)lelarge(dot)info>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Overhaul of type attributes modification
Date: 2011-07-09 20:11:51
Message-ID: CAA-aLv6BuGqO-7xgRbYpy+8AsC5iV0EceZaKsuiq7ymj7bqWfw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On 9 July 2011 21:07, Guillaume Lelarge <guillaume(at)lelarge(dot)info> wrote:
> On Sat, 2011-07-09 at 17:54 +0100, Thom Brown wrote:
>> On 9 July 2011 13:26, Thom Brown <thom(at)linux(dot)com> wrote:
>> > On 9 July 2011 13:25, Guillaume Lelarge <guillaume(at)lelarge(dot)info> wrote:
>> >> On Sat, 2011-07-09 at 13:14 +0100, Thom Brown wrote:
>> >>> > The major issue is why we use the "drop all, add all" method.  Let's say
>> >>> > I have a composite type declared like this:
>> >>> >
>> >>> > CREATE TYPE s1.ty2 AS
>> >>> >   (c1 integer,
>> >>> >    c2 integer,
>> >>> >    c3 text,
>> >>> >    c4 integer);
>> >>> >
>> >>> > If I change c3's type (but it could be c1 or c2), I get this SQL query:
>> >>> >
>> >>> > ALTER TYPE s1.ty2
>> >>> >  ADD ATTRIBUTE c3 xml;
>> >>> > ALTER TYPE s1.ty2
>> >>> >  DROP ATTRIBUTE c3;
>> >>> >
>> >>> > Remember that, on pgAdmin, it shows the list this way:
>> >>> >    c1 integer,
>> >>> >    c2 integer,
>> >>> >    c3 xml,
>> >>> >    c4 integer
>> >>> >
>> >>> > I click OK, and get back to the properties dialog. pgAdmin now shows the
>> >>> > list this way:
>> >>> >    c1 integer,
>> >>> >    c2 integer,
>> >>> >    c4 integer
>> >>> >    c3 xml,
>> >>> >
>> >>> > Which is true. We drop the attribute and add another one, which will be
>> >>> > at the end of the list.
>> >>>
>> >>> I have a solution.  It seems I overlooked the alter attribute
>> >>> capabilities.  We can just do:
>> >>>
>> >>> ALTER TYPE s1.ty2
>> >>> ALTER ATTRIBUTE c3 TYPE xml;
>> >>>
>> >>> That will preserve its position without ever having to drop it.  I'm
>> >>> not sure why I didn't see it before.
>> >>>
>> >>
>> >> Can you fix your patch to also use this syntax?
>> >
>> > Yes, I'll try to fix both issues.  Thanks for testing it.  I'll be
>> > back with a patch shortly.
>>
>> Okay, looks like it needed more work than I realised.  There are quite
>> a few extra changes in this one.  I've moved the various checks (such
>> as ensuring there are at least 2 attributes) out of the block for
>> creating a type so that the same rules apply to modifying types.  This
>> is because you shouldn't be able to modify a type so that there are
>> less than 2 attributes.
>>
>> But please try to break this.  I've tried various combinations of
>> actions (adding, renaming and changing type, just renaming, just
>> changing type, removing an original attribute, removing all and adding
>> back in, removing none and just adding additional ones) and can't find
>> anything wrong now.  But maybe there's something I missed.
>>
>> Revised patch attached.
>>
>
> Quick question, are you using the master branch, or 1.14? I keep getting
> issues when I apply this on 1.14. And it seems more a big issue to me,
> than a new feature.

I've always used master. I haven't honed my git-fu to the point of
working out how to switch branches. I should probably look that up.

--
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2011-07-09 22:48:20 Re: [FEATURE] Add schema option to all relevant objects
Previous Message Guillaume Lelarge 2011-07-09 20:07:02 Re: Overhaul of type attributes modification