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
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] |