Re: Current enums patch

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
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:23:28
Message-ID: 27552.1175545408@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

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?

I'm wondering why bother with the temp variable in cstring_enum,
as opposed to just "if (strlen(name) >= NAMEDATALEN)"?
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 */

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.

regards, tom lane

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2007-04-02 20:33:25 Bonjour patch
Previous Message Bruce Momjian 2007-04-02 20:13:23 Re: Concurrent psql v4 [WIP]