From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Tom Dunstan <pgsql(at)tomd(dot)cc>, Heikki Linnakangas <heikki(at)enterprisedb(dot)com>, pgsql-patches(at)postgresql(dot)org |
Subject: | Re: Current enums patch |
Date: | 2007-04-02 20:36:03 |
Message-ID: | 46116933.9060106@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2007-04-02 20:39:00 | Re: [pgsql-patches] O_DIRECT support for Windows |
Previous Message | Bruce Momjian | 2007-04-02 20:35:47 | Re: Dead Space Map version 3 (simplified) |