Hi folksHere's a first cut of the enums patch for feedback when people have time. It follows an anyenum pseudo-type approach as foreseen by Nostradamus in one of the original threads. (http://archives.postgresql.org/pgsql-hackers/2005-11/msg00457.php). That made the patch a little more intrusive than I had been hoping for, but hopefully the bits that touch core files like parse_coerce.c are easy to follow.
The patch is largely feature complete as far as what I wanted to achieve, although it's currently missing documentation patches. I'll add those at some point soonish. It's currently only been tested on my x86 FC5 box.
A few notes:- In much of the code, anyenum is treated identically to anyelement, except when trying to tie down generic parameters: at this point types which aren't enums will be rejected if the declared type is anyenum. Thus there are quite a few places in the patch where ANYENUMOID has just kinda been added in next to ANYELEMENTOID.
- The error messages in enum.c need some love. Apparently there's a document that I need to read in the sgml docco which explains error codes to me; currently I've got FIXME TOM comments in there.
- One difficulty that I had was trying to write a cast-text-to-enum function. While input functions pass their type oid as a second parameter, all that cast functions can get is the typmod. I did a bit of an ugly hack to pass the enum oid in as an int in that parameter if it's an enum type; it might be cleaner to allow cast functions that take an oid there rather than an int and pass the correct parameter depending on that type. I wasn't comfortable making a change like that in this patch without discussion, though.
- It appeared that to be cached in a syscache properly, the enum names had to be a Name/NameData thing rather than a text or whatever, so there's a limit of 64 characters per enum value. 64 characters should be enough for anybody :). Hmm, as I write this I realize that I should probably add some length checking to the create type function. And something prohibiting commas, since they're the array delimiter. Consider that on the TODO as well. :) Do we want to allow a customizable delimiter? IMO someone putting commas inside an enum value is doing something sick and twisted anyway, but maybe there's an argument to be made. They could always hack the delimiter on the array type, although that wouldn't survive a dump.
- You can't create generic anyenum functions in the PL's except for plpgsql. You can create functions taking/returning the concrete type, but not anyenum. I don't consider that a huge loss; we couldn't really think of a use case. I haven't added a regression test involving enums to e.g. plperl, although I have tested it with perlpython/tcl. Is that worth doing, or is it being paranoid?
- This patch requires an initdb, but I haven't bumped the catalog version number in the patch since I don't know when (if ever) it will be applied
Now a few questions from a first-time contributor:- Do I need to call CatalogUpdateIndexes() after I delete rows when dropping the type? Following the code for creating dropping standard user types, it's called on creation but not deletion.
- I'm a little unclear about when pfree() should be called. My understanding is that it will deallocate memory from the current memory context, but that context will be chucked once the current statement or whatever is dead. Is it just nice to do when we know that we're done with a particular chunk and to stop the context from growing, or are there other occasions? Or have I misunderstood what it does?
- Is pg_indent intended to be run by us mortals? I tried following the instructions in the README to make sure my indentation was kosher, but ran into issues with pgindent complaining about something called detab being missing. It's not on my Fedora box, and some quick Googling didn't unearth an obvious candidate, although a few dodgy looking scripts popped up. Is it a BSD tool?
Anyway, all comments / criticisms welcome. Have a look at the regression test to see what should be working.
Cheers Tom
enums-v1.patch.gz
Description: GNU Zip compressed data
---------------------------(end of broadcast)--------------------------- TIP 6: explain analyze is your friend
