Skip site navigation (1) Skip section navigation (2)

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 (view raw, whole thread or download thread mbox)
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

pgsql-patches by date

Next:From: Bruce MomjianDate: 2007-04-02 20:39:00
Subject: Re: [pgsql-patches] O_DIRECT support for Windows
Previous:From: Bruce MomjianDate: 2007-04-02 20:35:47
Subject: Re: Dead Space Map version 3 (simplified)

Privacy Policy | About PostgreSQL
Copyright © 1996-2017 The PostgreSQL Global Development Group