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
