Re: Current enums patch

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

In response to

Responses

Browse pgsql-patches by date

  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)