Re: [HACKERS] WIP: extensible enums

2010-11-14 Thread Josh Berkus
I'd say put it on and mark it with an [E]. We could use some more [E]asy items for that list. We don't need to add marginally-useful features just because they're easy. If it doesn't have a real use-case, the incremental maintenance cost of more code is a good reason to reject it. I'll

Re: [HACKERS] WIP: extensible enums

2010-11-13 Thread Peter Eisentraut
On fre, 2010-11-12 at 17:19 -0500, Robert Haas wrote: If we allow users to name objects, we ought to make every effort to also allow renaming them. In my mind, the only way renaming is too marginal to be useful is if the feature itself is too marginal to be useful. The bottom line is, any

Re: [HACKERS] WIP: extensible enums

2010-11-13 Thread Robert Haas
On Sat, Nov 13, 2010 at 12:30 PM, Peter Eisentraut pete...@gmx.net wrote: On fre, 2010-11-12 at 17:19 -0500, Robert Haas wrote: If we allow users to name objects, we ought to make every effort to also allow renaming them.  In my mind, the only way renaming is too marginal to be useful is if

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Bruce Momjian
Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: On 10/24/2010 08:12 PM, Tom Lane wrote: This shows that the bitmapset optimization really is quite effective, at least for cases where all the enum labels are sorted by OID after all. That motivated me to change the bitmapset

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Bruce Momjian
Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: On 10/24/2010 08:12 PM, Tom Lane wrote: This shows that the bitmapset optimization really is quite effective, at least for cases where all the enum labels are sorted by OID after all. That motivated me to change the bitmapset

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Andrew Dunstan
On 11/12/2010 01:40 PM, Bruce Momjian wrote: FYI, I marked the TODO item for adding enums as completed. The TODO item used to also mention renaming or removing enums, but I have seen few requests for that so I removed that suggestion. We can always re-add it if there is demand. Renaming

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 11/12/2010 01:40 PM, Bruce Momjian wrote: FYI, I marked the TODO item for adding enums as completed. The TODO item used to also mention renaming or removing enums, but I have seen few requests for that so I removed that suggestion. We can always

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Robert Haas
On Fri, Nov 12, 2010 at 1:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Dunstan and...@dunslane.net writes: On 11/12/2010 01:40 PM, Bruce Momjian wrote: FYI, I marked the TODO item for adding enums as completed.  The TODO item used to also mention renaming or removing enums, but I have seen

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Fri, Nov 12, 2010 at 1:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: Well, you can rename an item today if you don't mind doing a direct UPDATE on pg_enum.  I think that's probably sufficient if the demand only amounts to one or two requests a year.  

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Alvaro Herrera
Excerpts from Bruce Momjian's message of vie nov 12 15:40:28 -0300 2010: FYI, I marked the TODO item for adding enums as completed. The TODO item used to also mention renaming or removing enums, but I have seen few requests for that so I removed that suggestion. We can always re-add it if

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Robert Haas
On Nov 12, 2010, at 2:20 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Nov 12, 2010 at 1:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: Well, you can rename an item today if you don't mind doing a direct UPDATE on pg_enum. I think that's probably

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Joshua D. Drake
On Fri, 2010-11-12 at 14:20 -0500, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Nov 12, 2010 at 1:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: Well, you can rename an item today if you don't mind doing a direct UPDATE on pg_enum. I think that's probably sufficient if the

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Bruce Momjian
Joshua D. Drake wrote: On Fri, 2010-11-12 at 14:20 -0500, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Nov 12, 2010 at 1:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: Well, you can rename an item today if you don't mind doing a direct UPDATE on pg_enum. I think that's

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Andrew Dunstan
On 11/12/2010 09:18 PM, Bruce Momjian wrote: OK, got it. Added incomplete TODO item: Allow renaming and deleting enumerated values from an existing enumerated data type I have serious doubts that deleting will ever be sanely doable. cheers andrew -- Sent via

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Bruce Momjian
Andrew Dunstan wrote: On 11/12/2010 09:18 PM, Bruce Momjian wrote: OK, got it. Added incomplete TODO item: Allow renaming and deleting enumerated values from an existing enumerated data type I have serious doubts that deleting will ever be sanely doable. True. Should

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Greg Stark
On Sat, Oct 23, 2010 at 10:11 PM, Robert Haas robertmh...@gmail.com wrote: I dunno if floats have completely consistent representations on all the platforms we support, but with integers it should be quite easy to predict the exact point when you're going to run out of space and what the

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Dean Rasheed
On 24 October 2010 05:26, Tom Lane t...@sss.pgh.pa.us wrote: Well, the easy way to read a consistent view of the world is to load the cache using an MVCC snapshot instead of SnapshotNow.  The current code structure isn't amenable to that because it's relying on a syscache to fetch the data for

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Tom Lane
Greg Stark gsst...@mit.edu writes: There's nothing magic about the integral types here. If you use a string then you could always split by making the string longer. The entire objective here is to make enum comparisons fast. Somehow, going to a non-primitive data type to represent the sort

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Andrew Dunstan
On 10/24/2010 12:20 PM, Tom Lane wrote: With float4 the implementation would fail at somewhere around 2^24 elements in an enum (since even with renumbering, there wouldn't be enough bits to give each element a distinguishable value). I don't see that as a real objection, and anyway if you

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Dean Rasheed
On 24 October 2010 17:20, Tom Lane t...@sss.pgh.pa.us wrote: Greg Stark gsst...@mit.edu writes: There's nothing magic about the integral types here. If you use a string then you could always split by making the string longer. The entire objective here is to make enum comparisons fast.  

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com writes: The point with an OID array is that you wouldn't need to store the enumsortorder values at all. The sort order would just be the index of the OID in the array. So the comparison code would read the OID array, traverse it building an array of

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Andrew Dunstan
On 10/24/2010 03:33 PM, Tom Lane wrote: Dean Rasheeddean.a.rash...@gmail.com writes: The point with an OID array is that you wouldn't need to store the enumsortorder values at all. The sort order would just be the index of the OID in the array. So the comparison code would read the OID

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Andrew Dunstan
On 10/24/2010 03:28 PM, Tom Lane wrote: This patch isn't committable as-is because I haven't made enum_first, enum_last, enum_range follow these coding rules: they need to stop using the syscache and instead use an indexscan on pg_enum_typid_sortorder_index to locate the relevant rows. That

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Andrew Dunstan
On 10/24/2010 05:09 PM, Andrew Dunstan wrote: select enum_oid, row_number() over () as sort_order from unnest(null::myenum) as enum_oid Of course, I meant: select enum_label, row_number() over () as sort_order from unnest(enum_range(null::myenum)) as enum_label cheers

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Tom Lane
BTW, I've forgotten --- did anyone publish a test case for checking performance of enum comparisons? Or should I just cons up something privately? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Andrew Dunstan
On 10/24/2010 05:34 PM, Tom Lane wrote: BTW, I've forgotten --- did anyone publish a test case for checking performance of enum comparisons? Or should I just cons up something privately? I have been running tests with http://developer.postgresql.org/~adunstan/enumtest.dmp The table

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 10/24/2010 05:34 PM, Tom Lane wrote: BTW, I've forgotten --- did anyone publish a test case for checking performance of enum comparisons? Or should I just cons up something privately? I have been running tests with

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Andrew Dunstan
On 10/24/2010 08:12 PM, Tom Lane wrote: OK, I did some timing consisting of building a btree index with maintenance_work_mem set reasonably high. This is on a debug-enabled build, so it's not representative of production performance, but it will do for seeing what we're doing to enum

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 10/24/2010 08:12 PM, Tom Lane wrote: This shows that the bitmapset optimization really is quite effective, at least for cases where all the enum labels are sorted by OID after all. That motivated me to change the bitmapset setup code to what's

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Andrew Dunstan
On 10/24/2010 09:20 PM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: On 10/24/2010 08:12 PM, Tom Lane wrote: This shows that the bitmapset optimization really is quite effective, at least for cases where all the enum labels are sorted by OID after all. That motivated me to

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: Latest patch attached. I've been working through this patch. It occurs to me that there's a fairly serious problem with the current implementation of insertion of new values within the bounds of the current sort ordering. Namely, that it does that by

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Josh Berkus
The disadvantage of this scheme is that if you repeatedly insert entries in the same place in the sort order, you halve the available range each time, so you'd run out of room after order-of-fifty halvings. This is not a real issue. If anyone is using an ENUM with 1000 values in it, they're

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Josh Berkus j...@agliodbs.com writes: The disadvantage of this scheme is that if you repeatedly insert entries in the same place in the sort order, you halve the available range each time, so you'd run out of room after order-of-fifty halvings. This is not a real issue. If anyone is using an

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Robert Haas
On Oct 23, 2010, at 7:12 PM, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Dunstan and...@dunslane.net writes: Latest patch attached. I've been working through this patch. It occurs to me that there's a fairly serious problem with the current implementation of insertion of new values within

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Oct 23, 2010, at 7:12 PM, Tom Lane t...@sss.pgh.pa.us wrote: I've been working through this patch. It occurs to me that there's a fairly serious problem with the current implementation of insertion of new values within the bounds of the current

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Robert Haas
On Oct 23, 2010, at 7:52 PM, Tom Lane t...@sss.pgh.pa.us wrote: I still prefer the idea of not changing rows once they're inserted, though Me too. But I really dislike the idea of having a failure mode where we can't insert for no reason that the user can understand. So I'm trying to think

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: Why would you need to lock out type comparisons? Didn't you get the point? The hazard is to a concurrent process that is merely trying to load up its enum-values cache so that it can perform an enum comparison. I don't want such an operation to have to

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Andrew Dunstan
On 10/23/2010 07:12 PM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: Latest patch attached. I've been working through this patch. Cool. [snip: reallocating enum sortorder on existing values has race conditions. Suggestion to use a float8 instead and add new value half way

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: Seriously, I think it might be OK. Could we provide some safe way of resetting the sortorder values? Or even a not-entirely-safe superuser-only function might be useful. Binary upgrade could probably call it safely, for example. You could do it

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Robert Haas
On Sat, Oct 23, 2010 at 8:11 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Why would you need to lock out type comparisons? Didn't you get the point?  The hazard is to a concurrent process that is merely trying to load up its enum-values cache so that it can

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Andrew Dunstan
On 10/23/2010 08:54 PM, Tom Lane wrote: Another thought here is that the split-in-half rule might be unnecessarily dumb. It leaves equal amounts of code space on both sides of the new value, even though the odds of subsequent insertions on both sides are probably unequal. But I'm not sure if

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: I suppose you could fix this by always updating every row, and storing in each row the total count of elements (or a random number). Then it'd be obvious if you'd read an inconsistent view of the world. Well, the easy way to read a consistent view of

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Robert Haas
On Sun, Oct 24, 2010 at 12:26 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: I suppose you could fix this by always updating every row, and storing in each row the total count of elements (or a random number).  Then it'd be obvious if you'd read an

Re: [HACKERS] WIP: extensible enums

2010-10-20 Thread David Fetter
On Tue, Oct 19, 2010 at 08:51:16PM -0400, Robert Haas wrote: On Tue, Oct 19, 2010 at 5:42 PM, Andrew Dunstan and...@dunslane.net wrote: Well a bit more testing shows some benefit. I've sorted out a few kinks, so this seems to work. In particular, with the above tables, the version imported

Re: [HACKERS] WIP: extensible enums

2010-10-20 Thread Robert Haas
On Wed, Oct 20, 2010 at 3:16 PM, David Fetter da...@fetter.org wrote: On Tue, Oct 19, 2010 at 08:51:16PM -0400, Robert Haas wrote: On Tue, Oct 19, 2010 at 5:42 PM, Andrew Dunstan and...@dunslane.net wrote: Well a bit more testing shows some benefit. I've sorted out a few kinks, so this seems

Re: [HACKERS] WIP: extensible enums

2010-10-20 Thread Merlin Moncure
On Tue, Oct 19, 2010 at 9:15 PM, Andrew Dunstan and...@dunslane.net wrote: Efficiency has  always been one of the major reasons for using enums, so it's important that we make them extensible without badly affecting performance. on that note is it worthwhile backpatching recent versions to

Re: [HACKERS] WIP: extensible enums

2010-10-20 Thread Robert Haas
On Wed, Oct 20, 2010 at 6:54 PM, Merlin Moncure mmonc...@gmail.com wrote: On Tue, Oct 19, 2010 at 9:15 PM, Andrew Dunstan and...@dunslane.net wrote: Efficiency has  always been one of the major reasons for using enums, so it's important that we make them extensible without badly affecting

Re: [HACKERS] WIP: extensible enums

2010-10-19 Thread Dean Rasheed
On 19 October 2010 05:21, Andrew Dunstan and...@dunslane.net wrote: On 10/18/2010 10:52 AM, Tom Lane wrote: We could possibly deal with enum types that follow the existing convention if we made the cache entry hold a list of all the original, known-to-be-sorted OIDs.  (This could be

Re: [HACKERS] WIP: extensible enums

2010-10-19 Thread Thom Brown
On 19 October 2010 05:21, Andrew Dunstan and...@dunslane.net wrote: On 10/18/2010 10:52 AM, Tom Lane wrote: We could possibly deal with enum types that follow the existing convention if we made the cache entry hold a list of all the original, known-to-be-sorted OIDs.  (This could be

Re: [HACKERS] WIP: extensible enums

2010-10-19 Thread Magnus Hagander
On Tue, Oct 19, 2010 at 10:19, Thom Brown t...@linux.com wrote: On 19 October 2010 05:21, Andrew Dunstan and...@dunslane.net wrote: On 10/18/2010 10:52 AM, Tom Lane wrote: We could possibly deal with enum types that follow the existing convention if we made the cache entry hold a list of

Re: [HACKERS] WIP: extensible enums

2010-10-19 Thread Alvaro Herrera
Excerpts from Magnus Hagander's message of mar oct 19 05:23:31 -0300 2010: He certainly could, but github provides more features and a nicer look :-) And since it's git, it doesn't matter where the repo is. Yeah. If you have a checked out copy of the GIT repo (preferably one with the master

Re: [HACKERS] WIP: extensible enums

2010-10-19 Thread Andrew Dunstan
On 10/19/2010 12:21 AM, Andrew Dunstan wrote: On 10/18/2010 10:52 AM, Tom Lane wrote: We could possibly deal with enum types that follow the existing convention if we made the cache entry hold a list of all the original, known-to-be-sorted OIDs. (This could be reasonably compact and cheap

Re: [HACKERS] WIP: extensible enums

2010-10-19 Thread Robert Haas
On Tue, Oct 19, 2010 at 5:42 PM, Andrew Dunstan and...@dunslane.net wrote: Well a bit more testing shows some benefit. I've sorted out a few kinks, so this seems to work. In particular, with the above tables, the version imported from 9.0 can create have an index created in about the same time

Re: [HACKERS] WIP: extensible enums

2010-10-19 Thread Andrew Dunstan
On 10/19/2010 08:51 PM, Robert Haas wrote: On Tue, Oct 19, 2010 at 5:42 PM, Andrew Dunstanand...@dunslane.net wrote: Well a bit more testing shows some benefit. I've sorted out a few kinks, so this seems to work. In particular, with the above tables, the version imported from 9.0 can create

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Andrew Dunstan
On 10/17/2010 03:56 PM, Tom Lane wrote: [clever scheme to treat even numbered enum Oids as sorted] The one problem I can see with this is that it's only partially on-disk-compatible with existing enum types: it'll almost certainly think that they require slow comparison, even when they

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 10/17/2010 03:56 PM, Tom Lane wrote: [clever scheme to treat even numbered enum Oids as sorted] The one problem I can see with this is that it's only partially on-disk-compatible with existing enum types: it'll almost certainly think that they

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Andrew Dunstan
On 10/18/2010 10:52 AM, Tom Lane wrote: We could possibly deal with enum types that follow the existing convention if we made the cache entry hold a list of all the original, known-to-be-sorted OIDs. (This could be reasonably compact and cheap to probe if it were represented as a starting OID

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 10/18/2010 10:52 AM, Tom Lane wrote: We could possibly deal with enum types that follow the existing convention if we made the cache entry hold a list of all the original, known-to-be-sorted OIDs. (This could be reasonably compact and cheap to

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Dean Rasheed
On 18 October 2010 15:52, Tom Lane t...@sss.pgh.pa.us wrote: So I'm thinking the comparison procedure goes like this: 1. Both OIDs even?        If so, just compare them numerically, and we're done. 2. Lookup cache entry for enum type. 3. Both OIDs in list of known-sorted OIDs?        If

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com writes: I think the hash map lookups could be made pretty quick, if we're prepared to write a bit of dedicated code there rather than relying on the more general-purpose caching code. It's still going to be very significantly slower than what I'm

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Andrew Dunstan
On 10/18/2010 01:40 PM, Dean Rasheed wrote: On 18 October 2010 15:52, Tom Lanet...@sss.pgh.pa.us wrote: So I'm thinking the comparison procedure goes like this: 1. Both OIDs even? If so, just compare them numerically, and we're done. 2. Lookup cache entry for enum type. 3. Both

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: If you have want to work on it and prove it's going to be better, please do. I'm not convinced it will do a whole lot better than a binary search that in most cases will do no more than a handful of probes. Yeah, that's a good point. There's a

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Dean Rasheed
On 16 October 2010 18:25, Dean Rasheed dean.a.rash...@gmail.com wrote: Are there corner cases the author has failed to consider? None that I could find. Are there any assertion failures or crashes? I just thought of another corner case, which can lead to a crash. The comparison code assumes

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Andrew Dunstan
On 10/17/2010 05:30 AM, Dean Rasheed wrote: On 16 October 2010 18:25, Dean Rasheeddean.a.rash...@gmail.com wrote: Are there corner cases the author has failed to consider? None that I could find. Are there any assertion failures or crashes? I just thought of another corner case, which

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 10/17/2010 05:30 AM, Dean Rasheed wrote: I just thought of another corner case, which can lead to a crash. The comparison code assumes that the number of elements in the enumeration is constant during a query, but that's not necessarily the case.

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Dean Rasheed
On 17 October 2010 15:38, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Dunstan and...@dunslane.net writes: On 10/17/2010 05:30 AM, Dean Rasheed wrote: I just thought of another corner case, which can lead to a crash. The comparison code assumes that the number of elements in the enumeration is

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Andrew Dunstan
On 10/17/2010 10:38 AM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: On 10/17/2010 05:30 AM, Dean Rasheed wrote: I just thought of another corner case, which can lead to a crash. The comparison code assumes that the number of elements in the enumeration is constant during a

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com writes: Hmm, it's harder than I thought. The crash is because each time it finds a label it hasn't seen before it adds it to the array of cached values without checking the array bounds. That array is only as big as the number of elements in the enum the

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Dean Rasheed
On 17 October 2010 16:49, Tom Lane t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com writes: Hmm, it's harder than I thought. The crash is because each time it finds a label it hasn't seen before it adds it to the array of cached values without checking the array bounds. That

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com writes: On 17 October 2010 16:49, Tom Lane t...@sss.pgh.pa.us wrote: [ scratches head... ]  And where does it get that number of elements from, if not by doing the same work that would allow it to fill the array completely?  Something seems ill-designed

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
I wrote: The missing piece in this is how to determine when the typcache entry has to be flushed. That should be driven by sinval signalling. On reflection that doesn't seem good enough. Immediately after someone else has committed an ALTER TYPE, your typcache entry is out of date, and won't

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Dean Rasheed
On 17 October 2010 18:53, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: The missing piece in this is how to determine when the typcache entry has to be flushed.  That should be driven by sinval signalling. On reflection that doesn't seem good enough.  Immediately after someone else has

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Andrew Dunstan
On 10/17/2010 01:20 PM, Tom Lane wrote: I knew I shoulda read this patch ;-). That seems a lot more invasive than this feature justifies. And I share your qualms about whether it's race-condition-proof. We don't have very much locking on pg_type entries, so making a hard assumption about

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com writes: On 17 October 2010 18:53, Tom Lane t...@sss.pgh.pa.us wrote: We could fix that with Dean's idea of reloading the cache whenever we see that we are being asked to compare a value we don't have in the cache entry.  However, that assumes that we even

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Andrew Dunstan
On 10/17/2010 02:19 PM, Dean Rasheed wrote: That makes me think maybe the fast and slow comparisons should both be done the same way, having a cache so that we notice immediately if we get a new value. Obviously that's not going to be as fast as the current fast method, but the question is,

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Andrew Dunstan
On 10/17/2010 03:17 PM, Tom Lane wrote: Dean Rasheeddean.a.rash...@gmail.com writes: On 17 October 2010 18:53, Tom Lanet...@sss.pgh.pa.us wrote: We could fix that with Dean's idea of reloading the cache whenever we see that we are being asked to compare a value we don't have in the cache

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: Making that as fast as Is this sorted? If yes, compare the two oids or even acceptably slower seems likely to be a challenge. I thought about the sort of approach you suggest initially and didn't come up with anything that seemed likely to work

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 10/17/2010 03:17 PM, Tom Lane wrote: I think what this says is that we cannot allow any manipulations that involve an uncommitted enum value. Probably the easiest way is to make the ALTER TYPE operation disallowed-inside-transaction-block. That's

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 10/17/2010 01:20 PM, Tom Lane wrote: The way I'd be inclined to design this is that altering an enum doesn't change its pg_type entry at all, just add another row to pg_enum. When first needing to compare values of an enum, load up the typcache

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Andrew Dunstan
On 10/17/2010 03:56 PM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: Making that as fast as Is this sorted? If yes, compare the two oids or even acceptably slower seems likely to be a challenge. I thought about the sort of approach you suggest initially and didn't come up with

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Greg Stark
On Sun, Oct 17, 2010 at 12:17 PM, Tom Lane t...@sss.pgh.pa.us wrote:        begin;        alter type myenum add 'some-value';        insert into mytab values('some-value');        rollback; I think what this says is that we cannot allow any manipulations that involve an uncommitted enum

Re: [HACKERS] WIP: extensible enums

2010-10-16 Thread Dean Rasheed
On 10/15/2010 04:33 AM, Dean Rasheed wrote: I started looking at this last night, but ran out of time. I'll continue this evening / over the weekend. Continuing my review of this patch... Usability review What the patch does: This patch adds syntax to allow additional enum

Re: [HACKERS] WIP: extensible enums

2010-10-15 Thread Andrew Dunstan
On 10/15/2010 04:33 AM, Dean Rasheed wrote: I started looking at this last night, but ran out of time. I'll continue this evening / over the weekend. Here are my comments so far: Patch applies cleanly to current git master with no offsets. Compiles cleanly with no warnings. Regression tests

Re: [HACKERS] WIP: extensible enums

2010-10-14 Thread Dean Rasheed
On 13 October 2010 23:17, Robert Haas robertmh...@gmail.com wrote: On Wed, Oct 13, 2010 at 7:33 AM, Andrew Dunstan and...@dunslane.net wrote: Sorry, got distracted. Here's a new patch that fixes the above and also contains some documentation. Someone want to review this and (hopefully) mark

Re: [HACKERS] WIP: extensible enums

2010-10-14 Thread Robert Haas
On Wed, Oct 13, 2010 at 7:33 AM, Andrew Dunstan and...@dunslane.net wrote: Sorry, got distracted. Here's a new patch that fixes the above and also contains some documentation. Someone want to review this and (hopefully) mark it Ready for Committer? I see that Brendan Jurd is the reviewer of

Re: [HACKERS] WIP: extensible enums

2010-10-13 Thread Robert Haas
On Fri, Oct 1, 2010 at 7:12 AM, Andrew Dunstan and...@dunslane.net wrote: On 10/01/2010 04:35 AM, Dean Rasheed wrote: 2). In enum_ccmp(), when you cache the full list of elements, you're not updating mycache-sort_list_length, so it will keep fetching the full list each time. Also, I think

Re: [HACKERS] WIP: extensible enums

2010-10-01 Thread Dean Rasheed
On 29 September 2010 20:46, Andrew Dunstan and...@dunslane.net wrote: Attached is a a slightly updated version of this with the bitrot fixed. cheers andrew Hi, I had a quick look at this last night. I haven't had time to give it a full review, but I did spot a couple of things: 1). It

Re: [HACKERS] WIP: extensible enums

2010-10-01 Thread Andrew Dunstan
On 10/01/2010 04:35 AM, Dean Rasheed wrote: 2). In enum_ccmp(), when you cache the full list of elements, you're not updating mycache-sort_list_length, so it will keep fetching the full list each time. Also, I think that function could use a few more comments. Good catch. Will fix. 3). I

Re: [HACKERS] WIP: extensible enums

2010-08-25 Thread Andrew Dunstan
On 08/23/2010 07:34 PM, Bruce Momjian wrote: I looked at the pg_upgrade ramifications of this change and it seems some adjustments will have to be made. Right now pg_upgrade creates an empty enum type: CREATE TYPE etest AS ENUM (); and then it calls EnumValuesCreate() to create the

Re: [HACKERS] WIP: extensible enums

2010-08-24 Thread Andrew Dunstan
On 08/23/2010 07:12 PM, Bruce Momjian wrote: Josh Berkus wrote: On 8/23/10 12:20 PM, Tom Lane wrote: Josh Berkusj...@agliodbs.com writes: I really don't see the value in making a command substantially less intuitive in order to avoid a single keyword, unless it affects areas of Postgres

Re: [HACKERS] WIP: extensible enums

2010-08-24 Thread Bruce Momjian
Andrew Dunstan wrote: On 08/23/2010 07:12 PM, Bruce Momjian wrote: Josh Berkus wrote: On 8/23/10 12:20 PM, Tom Lane wrote: Josh Berkusj...@agliodbs.com writes: I really don't see the value in making a command substantially less intuitive in order to avoid a single keyword, unless

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Alvaro Herrera
Excerpts from Andrew Dunstan's message of lun ago 23 05:35:09 -0400 2010: To add a label at the end of the list, do: ALTER TYPE myenum ADD 'newlabel'; To add a label somewhere else, do: ALTER TYPE myenum ADD 'newlabel' BEFORE 'existinglabel'; or ALTER TYPE myenum ADD

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread David E. Wheeler
On Aug 23, 2010, at 2:35 AM, Andrew Dunstan wrote: I'm not wedded to the syntax. Let the bikeshedding begin. Seems pretty good to me as-is. David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Andrew Dunstan
On Mon, August 23, 2010 11:49 am, Alvaro Herrera wrote: Excerpts from Andrew Dunstan's message of lun ago 23 05:35:09 -0400 2010: To add a label at the end of the list, do: ALTER TYPE myenum ADD 'newlabel'; To add a label somewhere else, do: ALTER TYPE myenum ADD 'newlabel' BEFORE

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Andrew Dunstan
On Mon, August 23, 2010 11:49 am, Alvaro Herrera wrote: Excerpts from Andrew Dunstan's message of lun ago 23 05:35:09 -0400 2010: To add a label at the end of the list, do: ALTER TYPE myenum ADD 'newlabel'; To add a label somewhere else, do: ALTER TYPE myenum ADD 'newlabel' BEFORE

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On Mon, August 23, 2010 11:49 am, Alvaro Herrera wrote: What do you need AFTER for? Seems to me that BEFORE should be enough. (You already have the unadorned syntax for adding an item after the last one, which is the corner case that BEFORE alone

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Josh Berkus
You're right. Strictly speaking we don't need it. But it doesn't hurt much to provide it for a degree of symmetry. Swami Josh predicts that if we don't add AFTER now, we'll be adding it in 2 years when enough people complain about it. -- -- Josh Berkus

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Thom Brown
On 23 August 2010 10:35, Andrew Dunstan and...@dunslane.net wrote: Attached is a WIP patch that allows enums to be extended with additional labels arbitrarily. As previously discussed, it works by adding an explicit sort order column to pg_enum. It keeps track of whether the labels are

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread David Fetter
On Mon, Aug 23, 2010 at 11:49:41AM -0400, Alvaro Herrera wrote: Excerpts from Andrew Dunstan's message of lun ago 23 05:35:09 -0400 2010: To add a label at the end of the list, do: ALTER TYPE myenum ADD 'newlabel'; To add a label somewhere else, do: ALTER TYPE myenum ADD

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread David Fetter
On Mon, Aug 23, 2010 at 01:54:40PM -0400, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: On Mon, August 23, 2010 11:49 am, Alvaro Herrera wrote: What do you need AFTER for? Seems to me that BEFORE should be enough. (You already have the unadorned syntax for adding an item

  1   2   >