Thread: Overhaul of type attributes modification

Hi all,

I noticed that if you add, delete, rename or change the type,
collation or precision of a composite type attribute, it deletes all
of them then adds them all back in.  Obviously attribute additions,
deletions and modifications may only occur for types under PostgreSQL
9.1, but I thought it a bit extreme to actually drop all types if
you're adding a new one in, or just removing one.  Also, if you rename
a type, it drops all attributes again then re-adds them, just to have
an attribute with a different name.

So I've attached a patch which "does the right thing".  Take the
example of the following type:

CREATE TYPE bark AS
   (one text,
    two text,
    three text,
    four text,
    five text);

Say we wanted to remove "two", change the type of three to uuid,
rename four to forty and add an extra text type of six.

Normally we'd just get:

ALTER TYPE bark DROP ATTRIBUTE one;
ALTER TYPE bark DROP ATTRIBUTE two;
ALTER TYPE bark DROP ATTRIBUTE three;
ALTER TYPE bark DROP ATTRIBUTE four;
ALTER TYPE bark DROP ATTRIBUTE five;
ALTER TYPE bark ADD ATTRIBUTE one text;
ALTER TYPE bark ADD ATTRIBUTE three text;
ALTER TYPE bark ADD ATTRIBUTE forty uuid;
ALTER TYPE bark ADD ATTRIBUTE five text;
ALTER TYPE bark ADD ATTRIBUTE six uuid;

With these changes we'd now get:

ALTER TYPE bark DROP ATTRIBUTE two;
ALTER TYPE bark RENAME ATTRIBUTE four TO forty;
ALTER TYPE bark ADD ATTRIBUTE six text;

.. except now those are also nicely indented to be more readable for
types with long schemas/type names.

e.g.
ALTER TYPE long_schema_name.quite_a_long_table_name
  ADD ATTRIBUTE "suspiciously long attribute name"?

It also fixes a bug whereby if you have a precision specified, the
word COLLATE mysteriously appears after the type whether or not you
have a collation assigned because the collation condition was based on
the precision being present for some reason.  And if you actually
assigned a collation for a valid data type, it wouldn't appear at all,
so that's fixed too.

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

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

On 8 July 2011 15:19, Thom Brown <thom@linux.com> wrote:
> Hi all,
>
> I noticed that if you add, delete, rename or change the type,
> collation or precision of a composite type attribute, it deletes all
> of them then adds them all back in.  Obviously attribute additions,
> deletions and modifications may only occur for types under PostgreSQL
> 9.1, but I thought it a bit extreme to actually drop all types if
> you're adding a new one in, or just removing one.  Also, if you rename
> a type, it drops all attributes again then re-adds them, just to have
> an attribute with a different name.
>
> So I've attached a patch which "does the right thing".  Take the
> example of the following type:
>
> CREATE TYPE bark AS
>   (one text,
>    two text,
>    three text,
>    four text,
>    five text);
>
> Say we wanted to remove "two", change the type of three to uuid,
> rename four to forty and add an extra text type of six.
>
> Normally we'd just get:
>
> ALTER TYPE bark DROP ATTRIBUTE one;
> ALTER TYPE bark DROP ATTRIBUTE two;
> ALTER TYPE bark DROP ATTRIBUTE three;
> ALTER TYPE bark DROP ATTRIBUTE four;
> ALTER TYPE bark DROP ATTRIBUTE five;
> ALTER TYPE bark ADD ATTRIBUTE one text;
> ALTER TYPE bark ADD ATTRIBUTE three text;
> ALTER TYPE bark ADD ATTRIBUTE forty uuid;
> ALTER TYPE bark ADD ATTRIBUTE five text;
> ALTER TYPE bark ADD ATTRIBUTE six uuid;
>
> With these changes we'd now get:
>
> ALTER TYPE bark DROP ATTRIBUTE two;
> ALTER TYPE bark RENAME ATTRIBUTE four TO forty;
> ALTER TYPE bark ADD ATTRIBUTE six text;
>
> .. except now those are also nicely indented to be more readable for
> types with long schemas/type names.
>
> e.g.
> ALTER TYPE long_schema_name.quite_a_long_table_name
>  ADD ATTRIBUTE "suspiciously long attribute name"?
>
> It also fixes a bug whereby if you have a precision specified, the
> word COLLATE mysteriously appears after the type whether or not you
> have a collation assigned because the collation condition was based on
> the precision being present for some reason.  And if you actually
> assigned a collation for a valid data type, it wouldn't appear at all,
> so that's fixed too.

Actually I noticed that the example I gave of what would occur before
the patch highlights another bug.  Notice that forty and six have the
type of uuid, and three doesn't?  Three should have been the one with
uuid, and forty and sixty should have been text.  That's not my typo
as that's come from PgAdmin 1.14 beta 2.  So it was actually assigning
the wrong data types in those case too.  So the patch fixes those
problems too.

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

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

On 8 July 2011 15:38, Thom Brown <thom@linux.com> wrote:
> On 8 July 2011 15:19, Thom Brown <thom@linux.com> wrote:
>> Hi all,
>>
>> I noticed that if you add, delete, rename or change the type,
>> collation or precision of a composite type attribute, it deletes all
>> of them then adds them all back in.  Obviously attribute additions,
>> deletions and modifications may only occur for types under PostgreSQL
>> 9.1, but I thought it a bit extreme to actually drop all types if
>> you're adding a new one in, or just removing one.  Also, if you rename
>> a type, it drops all attributes again then re-adds them, just to have
>> an attribute with a different name.
>>
>> So I've attached a patch which "does the right thing".  Take the
>> example of the following type:
>>
>> CREATE TYPE bark AS
>>   (one text,
>>    two text,
>>    three text,
>>    four text,
>>    five text);
>>
>> Say we wanted to remove "two", change the type of three to uuid,
>> rename four to forty and add an extra text type of six.
>>
>> Normally we'd just get:
>>
>> ALTER TYPE bark DROP ATTRIBUTE one;
>> ALTER TYPE bark DROP ATTRIBUTE two;
>> ALTER TYPE bark DROP ATTRIBUTE three;
>> ALTER TYPE bark DROP ATTRIBUTE four;
>> ALTER TYPE bark DROP ATTRIBUTE five;
>> ALTER TYPE bark ADD ATTRIBUTE one text;
>> ALTER TYPE bark ADD ATTRIBUTE three text;
>> ALTER TYPE bark ADD ATTRIBUTE forty uuid;
>> ALTER TYPE bark ADD ATTRIBUTE five text;
>> ALTER TYPE bark ADD ATTRIBUTE six uuid;
>>
>> With these changes we'd now get:
>>
>> ALTER TYPE bark DROP ATTRIBUTE two;
>> ALTER TYPE bark RENAME ATTRIBUTE four TO forty;
>> ALTER TYPE bark ADD ATTRIBUTE six text;
>>
>> .. except now those are also nicely indented to be more readable for
>> types with long schemas/type names.
>>
>> e.g.
>> ALTER TYPE long_schema_name.quite_a_long_table_name
>>  ADD ATTRIBUTE "suspiciously long attribute name"?
>>
>> It also fixes a bug whereby if you have a precision specified, the
>> word COLLATE mysteriously appears after the type whether or not you
>> have a collation assigned because the collation condition was based on
>> the precision being present for some reason.  And if you actually
>> assigned a collation for a valid data type, it wouldn't appear at all,
>> so that's fixed too.
>
> Actually I noticed that the example I gave of what would occur before
> the patch highlights another bug.  Notice that forty and six have the
> type of uuid, and three doesn't?  Three should have been the one with
> uuid, and forty and sixty should have been text.  That's not my typo
> as that's come from PgAdmin 1.14 beta 2.  So it was actually assigning
> the wrong data types in those case too.  So the patch fixes those
> problems too.

Also I noticed that my example of the new output doesn't show the
three datatype being changed to uuid.  This was just me forgetting to
click the Change button when I changed the type, so it would actually
come out as:

ALTER TYPE bark DROP ATTRIBUTE two;
ALTER TYPE bark DROP ATTRIBUTE three;
ALTER TYPE bark ADD ATTRIBUTE three uuid;
ALTER TYPE bark RENAME ATTRIBUTE four TO forty;
ALTER TYPE bark ADD ATTRIBUTE six text;

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

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

On Fri, 2011-07-08 at 16:01 +0100, Thom Brown wrote:
> On 8 July 2011 15:38, Thom Brown <thom@linux.com> wrote:
> > On 8 July 2011 15:19, Thom Brown <thom@linux.com> wrote:
> >> Hi all,
> >>
> >> I noticed that if you add, delete, rename or change the type,
> >> collation or precision of a composite type attribute, it deletes all
> >> of them then adds them all back in.  Obviously attribute additions,
> >> deletions and modifications may only occur for types under PostgreSQL
> >> 9.1, but I thought it a bit extreme to actually drop all types if
> >> you're adding a new one in, or just removing one.  Also, if you rename
> >> a type, it drops all attributes again then re-adds them, just to have
> >> an attribute with a different name.
> >>
> >> So I've attached a patch which "does the right thing".  Take the
> >> example of the following type:
> >>
> >> CREATE TYPE bark AS
> >>   (one text,
> >>    two text,
> >>    three text,
> >>    four text,
> >>    five text);
> >>
> >> Say we wanted to remove "two", change the type of three to uuid,
> >> rename four to forty and add an extra text type of six.
> >>
> >> Normally we'd just get:
> >>
> >> ALTER TYPE bark DROP ATTRIBUTE one;
> >> ALTER TYPE bark DROP ATTRIBUTE two;
> >> ALTER TYPE bark DROP ATTRIBUTE three;
> >> ALTER TYPE bark DROP ATTRIBUTE four;
> >> ALTER TYPE bark DROP ATTRIBUTE five;
> >> ALTER TYPE bark ADD ATTRIBUTE one text;
> >> ALTER TYPE bark ADD ATTRIBUTE three text;
> >> ALTER TYPE bark ADD ATTRIBUTE forty uuid;
> >> ALTER TYPE bark ADD ATTRIBUTE five text;
> >> ALTER TYPE bark ADD ATTRIBUTE six uuid;
> >>
> >> With these changes we'd now get:
> >>
> >> ALTER TYPE bark DROP ATTRIBUTE two;
> >> ALTER TYPE bark RENAME ATTRIBUTE four TO forty;
> >> ALTER TYPE bark ADD ATTRIBUTE six text;
> >>
> >> .. except now those are also nicely indented to be more readable for
> >> types with long schemas/type names.
> >>
> >> e.g.
> >> ALTER TYPE long_schema_name.quite_a_long_table_name
> >>  ADD ATTRIBUTE "suspiciously long attribute name"?
> >>
> >> It also fixes a bug whereby if you have a precision specified, the
> >> word COLLATE mysteriously appears after the type whether or not you
> >> have a collation assigned because the collation condition was based on
> >> the precision being present for some reason.  And if you actually
> >> assigned a collation for a valid data type, it wouldn't appear at all,
> >> so that's fixed too.
> >
> > Actually I noticed that the example I gave of what would occur before
> > the patch highlights another bug.  Notice that forty and six have the
> > type of uuid, and three doesn't?  Three should have been the one with
> > uuid, and forty and sixty should have been text.  That's not my typo
> > as that's come from PgAdmin 1.14 beta 2.  So it was actually assigning
> > the wrong data types in those case too.  So the patch fixes those
> > problems too.
>
> Also I noticed that my example of the new output doesn't show the
> three datatype being changed to uuid.  This was just me forgetting to
> click the Change button when I changed the type, so it would actually
> come out as:
>
> ALTER TYPE bark DROP ATTRIBUTE two;
> ALTER TYPE bark DROP ATTRIBUTE three;
> ALTER TYPE bark ADD ATTRIBUTE three uuid;
> ALTER TYPE bark RENAME ATTRIBUTE four TO forty;
> ALTER TYPE bark ADD ATTRIBUTE six text;
>

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 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.

One solution, if we don't want to keep the "drop all, add all" method,
is that an existing member with a new type or collation should always be
added at the end of the list. Another solution, we can allow someone to
change a member position. PostgreSQL doesn't allow that, but pgAdmin
could do it with ADD and DROP nicely combined. But I guess the
GetSqlForTypes() method will be pretty hard to write.


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

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.

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

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

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

On 9 July 2011 13:25, Guillaume Lelarge <guillaume@lelarge.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.

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

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

On 9 July 2011 13:26, Thom Brown <thom@linux.com> wrote:
> On 9 July 2011 13:25, Guillaume Lelarge <guillaume@lelarge.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.

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

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

On Sat, 2011-07-09 at 17:54 +0100, Thom Brown wrote:
> On 9 July 2011 13:26, Thom Brown <thom@linux.com> wrote:
> > On 9 July 2011 13:25, Guillaume Lelarge <guillaume@lelarge.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.


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

On 9 July 2011 21:07, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> On Sat, 2011-07-09 at 17:54 +0100, Thom Brown wrote:
>> On 9 July 2011 13:26, Thom Brown <thom@linux.com> wrote:
>> > On 9 July 2011 13:25, Guillaume Lelarge <guillaume@lelarge.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

On 9 July 2011 21:11, Thom Brown <thom@linux.com> wrote:
> On 9 July 2011 21:07, Guillaume Lelarge <guillaume@lelarge.info> wrote:
>> On Sat, 2011-07-09 at 17:54 +0100, Thom Brown wrote:
>>> On 9 July 2011 13:26, Thom Brown <thom@linux.com> wrote:
>>> > On 9 July 2011 13:25, Guillaume Lelarge <guillaume@lelarge.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

On 9 July 2011 23:58, Thom Brown <thom@linux.com> wrote:
> 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

Okay, here are the two rebased patches, both with the extra fix:

Patch for REL-1_14_0_PATCHES:
update_composite_type_attributes_sql_v2_rebased_1.14.patch

Patch for master: update_composite_type_attributes_sql_v2_rebased_master.patch

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

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

On Sun, 2011-07-10 at 01:05 +0100, Thom Brown wrote:
> On 9 July 2011 23:58, Thom Brown <thom@linux.com> wrote:
> > 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
>
> Okay, here are the two rebased patches, both with the extra fix:
>
> Patch for REL-1_14_0_PATCHES:
> update_composite_type_attributes_sql_v2_rebased_1.14.patch
>
> Patch for master: update_composite_type_attributes_sql_v2_rebased_master.patch
>

Thanks. Can't reproduce the bugs I found earlier, but found a new one.

Suppose the following type:

CREATE TYPE s1.ty3 AS
   (c1 uuid,
    c2 uuid);

If I remove the first element, the SQL textbox is empty. It will
probably help you to know that I get the same behaviour when I delete
the three first elements on a 4-elements type, but not if I delete less
than three for this type.

Sorry. Hope you won't kill me next time we meet :)


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

On 11 July 2011 15:21, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> On Sun, 2011-07-10 at 01:05 +0100, Thom Brown wrote:
>> On 9 July 2011 23:58, Thom Brown <thom@linux.com> wrote:
>> > 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
>>
>> Okay, here are the two rebased patches, both with the extra fix:
>>
>> Patch for REL-1_14_0_PATCHES:
>> update_composite_type_attributes_sql_v2_rebased_1.14.patch
>>
>> Patch for master: update_composite_type_attributes_sql_v2_rebased_master.patch
>>
>
> Thanks. Can't reproduce the bugs I found earlier, but found a new one.
>
> Suppose the following type:
>
> CREATE TYPE s1.ty3 AS
>   (c1 uuid,
>    c2 uuid);
>
> If I remove the first element, the SQL textbox is empty. It will
> probably help you to know that I get the same behaviour when I delete
> the three first elements on a 4-elements type, but not if I delete less
> than three for this type.
>
> Sorry. Hope you won't kill me next time we meet :)

No, that's not a bug.  That's something I changed.  This is what I
posted before:

"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."

If my assumption is flawed, please feel free to remove that part of
the change.  In fact I'm wondering if we should have that restriction
for either case (CREATE and ALTER) since you can actually create
composite types with no sub-types within them.  I just wanted to unify
the logic between the two.

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

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

On Mon, 2011-07-11 at 15:29 +0100, Thom Brown wrote:
> On 11 July 2011 15:21, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> > On Sun, 2011-07-10 at 01:05 +0100, Thom Brown wrote:
> >> On 9 July 2011 23:58, Thom Brown <thom@linux.com> wrote:
> >> > 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
> >>
> >> Okay, here are the two rebased patches, both with the extra fix:
> >>
> >> Patch for REL-1_14_0_PATCHES:
> >> update_composite_type_attributes_sql_v2_rebased_1.14.patch
> >>
> >> Patch for master: update_composite_type_attributes_sql_v2_rebased_master.patch
> >>
> >
> > Thanks. Can't reproduce the bugs I found earlier, but found a new one.
> >
> > Suppose the following type:
> >
> > CREATE TYPE s1.ty3 AS
> >   (c1 uuid,
> >    c2 uuid);
> >
> > If I remove the first element, the SQL textbox is empty. It will
> > probably help you to know that I get the same behaviour when I delete
> > the three first elements on a 4-elements type, but not if I delete less
> > than three for this type.
> >
> > Sorry. Hope you won't kill me next time we meet :)
>
> No, that's not a bug.  That's something I changed.  This is what I
> posted before:
>
> "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."
>

Oh OK, you're definitely right. Sorry about this.

I don't like the checks but, if they were already there, it makes sense
to apply them on creation and alteration of a type.

> If my assumption is flawed, please feel free to remove that part of
> the change.  In fact I'm wondering if we should have that restriction
> for either case (CREATE and ALTER) since you can actually create
> composite types with no sub-types within them.  I just wanted to unify
> the logic between the two.
>

And that's a good idea.

Well, your patch seems good to me. I've commited the bug fix (on the
collation clause) on the 1.14 master, and the complete patch on the
master branch. The patch is pretty invasive for something that's not a
bugfix, so I don't commit the complete patch to the 1.14 branch. Dave
and Thom, maybe you have a different opinion on this?


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

On Thu, Jul 14, 2011 at 10:22 AM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> On Mon, 2011-07-11 at 15:29 +0100, Thom Brown wrote:
>> On 11 July 2011 15:21, Guillaume Lelarge <guillaume@lelarge.info> wrote:
>> > On Sun, 2011-07-10 at 01:05 +0100, Thom Brown wrote:
>> >> On 9 July 2011 23:58, Thom Brown <thom@linux.com> wrote:
>> >> > 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
>> >>
>> >> Okay, here are the two rebased patches, both with the extra fix:
>> >>
>> >> Patch for REL-1_14_0_PATCHES:
>> >> update_composite_type_attributes_sql_v2_rebased_1.14.patch
>> >>
>> >> Patch for master: update_composite_type_attributes_sql_v2_rebased_master.patch
>> >>
>> >
>> > Thanks. Can't reproduce the bugs I found earlier, but found a new one.
>> >
>> > Suppose the following type:
>> >
>> > CREATE TYPE s1.ty3 AS
>> >   (c1 uuid,
>> >    c2 uuid);
>> >
>> > If I remove the first element, the SQL textbox is empty. It will
>> > probably help you to know that I get the same behaviour when I delete
>> > the three first elements on a 4-elements type, but not if I delete less
>> > than three for this type.
>> >
>> > Sorry. Hope you won't kill me next time we meet :)
>>
>> No, that's not a bug.  That's something I changed.  This is what I
>> posted before:
>>
>> "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."
>>
>
> Oh OK, you're definitely right. Sorry about this.
>
> I don't like the checks but, if they were already there, it makes sense
> to apply them on creation and alteration of a type.
>
>> If my assumption is flawed, please feel free to remove that part of
>> the change.  In fact I'm wondering if we should have that restriction
>> for either case (CREATE and ALTER) since you can actually create
>> composite types with no sub-types within them.  I just wanted to unify
>> the logic between the two.
>>
>
> And that's a good idea.
>
> Well, your patch seems good to me. I've commited the bug fix (on the
> collation clause) on the 1.14 master, and the complete patch on the
> master branch. The patch is pretty invasive for something that's not a
> bugfix, so I don't commit the complete patch to the 1.14 branch. Dave
> and Thom, maybe you have a different opinion on this?

I'd rather keep it for master only.


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

On 14 July 2011 10:28, Dave Page <dpage@pgadmin.org> wrote:
> On Thu, Jul 14, 2011 at 10:22 AM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> On Mon, 2011-07-11 at 15:29 +0100, Thom Brown wrote:
>>> On 11 July 2011 15:21, Guillaume Lelarge <guillaume@lelarge.info> wrote:
>>> > On Sun, 2011-07-10 at 01:05 +0100, Thom Brown wrote:
>>> >> On 9 July 2011 23:58, Thom Brown <thom@linux.com> wrote:
>>> >> > 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
>>> >>
>>> >> Okay, here are the two rebased patches, both with the extra fix:
>>> >>
>>> >> Patch for REL-1_14_0_PATCHES:
>>> >> update_composite_type_attributes_sql_v2_rebased_1.14.patch
>>> >>
>>> >> Patch for master: update_composite_type_attributes_sql_v2_rebased_master.patch
>>> >>
>>> >
>>> > Thanks. Can't reproduce the bugs I found earlier, but found a new one.
>>> >
>>> > Suppose the following type:
>>> >
>>> > CREATE TYPE s1.ty3 AS
>>> >   (c1 uuid,
>>> >    c2 uuid);
>>> >
>>> > If I remove the first element, the SQL textbox is empty. It will
>>> > probably help you to know that I get the same behaviour when I delete
>>> > the three first elements on a 4-elements type, but not if I delete less
>>> > than three for this type.
>>> >
>>> > Sorry. Hope you won't kill me next time we meet :)
>>>
>>> No, that's not a bug.  That's something I changed.  This is what I
>>> posted before:
>>>
>>> "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."
>>>
>>
>> Oh OK, you're definitely right. Sorry about this.
>>
>> I don't like the checks but, if they were already there, it makes sense
>> to apply them on creation and alteration of a type.
>>
>>> If my assumption is flawed, please feel free to remove that part of
>>> the change.  In fact I'm wondering if we should have that restriction
>>> for either case (CREATE and ALTER) since you can actually create
>>> composite types with no sub-types within them.  I just wanted to unify
>>> the logic between the two.
>>>
>>
>> And that's a good idea.
>>
>> Well, your patch seems good to me. I've commited the bug fix (on the
>> collation clause) on the 1.14 master, and the complete patch on the
>> master branch. The patch is pretty invasive for something that's not a
>> bugfix, so I don't commit the complete patch to the 1.14 branch. Dave
>> and Thom, maybe you have a different opinion on this?
>
> I'd rather keep it for master only.

If that's the case, someone will need to fix the bugs in 1.14 where
changing attributes results in possibly using the wrong data types in
the add attribute definitions as I posted in my 2nd message.

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

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

On Thu, 2011-07-14 at 12:30 +0100, Thom Brown wrote:
> On 14 July 2011 10:28, Dave Page <dpage@pgadmin.org> wrote:
> > On Thu, Jul 14, 2011 at 10:22 AM, Guillaume Lelarge
> > <guillaume@lelarge.info> wrote:
> >> On Mon, 2011-07-11 at 15:29 +0100, Thom Brown wrote:
> >>> On 11 July 2011 15:21, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> >>> > On Sun, 2011-07-10 at 01:05 +0100, Thom Brown wrote:
> >>> >> On 9 July 2011 23:58, Thom Brown <thom@linux.com> wrote:
> >>> >> > 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
> >>> >>
> >>> >> Okay, here are the two rebased patches, both with the extra fix:
> >>> >>
> >>> >> Patch for REL-1_14_0_PATCHES:
> >>> >> update_composite_type_attributes_sql_v2_rebased_1.14.patch
> >>> >>
> >>> >> Patch for master: update_composite_type_attributes_sql_v2_rebased_master.patch
> >>> >>
> >>> >
> >>> > Thanks. Can't reproduce the bugs I found earlier, but found a new one.
> >>> >
> >>> > Suppose the following type:
> >>> >
> >>> > CREATE TYPE s1.ty3 AS
> >>> >   (c1 uuid,
> >>> >    c2 uuid);
> >>> >
> >>> > If I remove the first element, the SQL textbox is empty. It will
> >>> > probably help you to know that I get the same behaviour when I delete
> >>> > the three first elements on a 4-elements type, but not if I delete less
> >>> > than three for this type.
> >>> >
> >>> > Sorry. Hope you won't kill me next time we meet :)
> >>>
> >>> No, that's not a bug.  That's something I changed.  This is what I
> >>> posted before:
> >>>
> >>> "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."
> >>>
> >>
> >> Oh OK, you're definitely right. Sorry about this.
> >>
> >> I don't like the checks but, if they were already there, it makes sense
> >> to apply them on creation and alteration of a type.
> >>
> >>> If my assumption is flawed, please feel free to remove that part of
> >>> the change.  In fact I'm wondering if we should have that restriction
> >>> for either case (CREATE and ALTER) since you can actually create
> >>> composite types with no sub-types within them.  I just wanted to unify
> >>> the logic between the two.
> >>>
> >>
> >> And that's a good idea.
> >>
> >> Well, your patch seems good to me. I've commited the bug fix (on the
> >> collation clause) on the 1.14 master, and the complete patch on the
> >> master branch. The patch is pretty invasive for something that's not a
> >> bugfix, so I don't commit the complete patch to the 1.14 branch. Dave
> >> and Thom, maybe you have a different opinion on this?
> >
> > I'd rather keep it for master only.
>
> If that's the case, someone will need to fix the bugs in 1.14 where
> changing attributes results in possibly using the wrong data types in
> the add attribute definitions as I posted in my 2nd message.
>

I don't have the issue at all. I tried a few other things, I looked into
the code, nothing feels wrong to me. Must have been fixed somehow, I
guess.


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