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

Enums patch v1

From: Tom Dunstan <pgsql(at)tomd(dot)cc>
To: pgsql-patches(at)postgresql(dot)org
Subject: Enums patch v1
Date: 2006-08-31 16:09:34
Message-ID: 44F709BE.50700@tomd.cc (view raw or flat)
Thread:
Lists: pgsql-patches
Hi folks

Here'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

Attachment: enums-v1.patch.gz
Description: application/x-gzip (20.3 KB)

pgsql-patches by date

Next:From: Guillaume SmetDate: 2006-08-31 17:33:14
Subject: Re: [HACKERS] [PATCHES] log_statement output for protocol
Previous:From: Tom LaneDate: 2006-08-31 15:10:47
Subject: Re: [HACKERS] Updatable views

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