Re: Overhaul of type attributes modification

On Sat, 2011-07-09 at 13:14 +0100, Thom Brown wrote:
> On 9 July 2011 12:52, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> > I took a look at this patch. The new function is really good (I love the
> > "hold" trick), but I have found two issues, one major and one minor.
> >
> > The minor issue is simple, and could probably get fixed easily. 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 remove c4, and change c3's name and type (say c3b and xml type), I
> > get this SQL query:
> >
> > ALTER TYPE s1.ty2
> >  ADD ATTRIBUTE c3b xml;
> > ALTER TYPE s1.ty2
> >  DROP ATTRIBUTE c3;
>
> I'm not sure how it could do that as it doesn't do that for me.  If an
> attribute changes which isn't the name, the code shows that it should
> drop the attribute then add it back in.  It's very confusing that the
> order is now wrong.  But yes, there's definitely a bug in that it
> didn't remove c4.
>
> > I'm not sure why, but it forgets about the c4 removal. I kind of fixed
> > it, but then I stumbled onto another issue, the major one.
> >
> > 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?


--
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com