Re: Overhaul of type attributes modification

On Thu, Jul 14, 2011 at 10:22 AM, Guillaume Lelarge
<guillaume(at)lelarge(dot)info> wrote:
> On Mon, 2011-07-11 at 15:29 +0100, Thom Brown wrote:
>> On 11 July 2011 15:21, Guillaume Lelarge <guillaume(at)lelarge(dot)info> wrote:
>> > On Sun, 2011-07-10 at 01:05 +0100, Thom Brown wrote:
>> >> On 9 July 2011 23:58, Thom Brown <thom(at)linux(dot)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