Re: Overhaul of type attributes modification

On 14 July 2011 10:28, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> 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.

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