Re: Overhaul of type attributes modification

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?

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