Re: Current enums patch

Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>> Working patch attached. If everyone's happy I'll apply it.
>>
>
> Why not put the create-time length test into EnumValuesCreate's loop,
> which has to grovel through the list already?
>

My idea was to do the error check before calling TypeCreate. If that
doesn't matter I can move it - it just seemed a bit cleaner to do it
that way than to have to roll back a change to pg_type.

> I'm wondering why bother with the temp variable in cstring_enum,
> as opposed to just "if (strlen(name) >= NAMEDATALEN)"?
>

Originally I was going to use the length in the message. But I have
changed it now.

> Also, a comment about why the test is necessary seems appropriate,
> since otherwise someone might think it redundant:
>     /* must check length to prevent Assert failure within SearchSysCache */
>

OK
> Lastly, a three-part regression test for this seems a bit silly.
> Or a lot silly.  If we added test cases for every niggling little
> bug we fix, the regression tests would be taking an hour to run
> and would be less productive, not more, because people wouldn't
> run them as often.
>
>

OK.

cheers

andrew