Re: Overhaul of type attributes modification

On 9 July 2011 21:11, Thom Brown <thom(at)linux(dot)com> wrote:
> 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.

I've found a corner-case bug btw, which requires a tiny amendment.
But I'm also rebasing the patch for current master and 1.14_patches
(or whatever it's called), so you'll get these shortly

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

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