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.

Re: [HACKERS] WIP: extensible enums

2010-11-13 Thread Robert Haas
On Sat, Nov 13, 2010 at 12:30 PM, Peter Eisentraut 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 the feature

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

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 pgsql-hacke

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 writes: > > > On Fri, Nov 12, 2010 at 1:55 PM, Tom Lane 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 Joshua D. Drake
On Fri, 2010-11-12 at 14:20 -0500, Tom Lane wrote: > Robert Haas writes: > > On Fri, Nov 12, 2010 at 1:55 PM, Tom Lane 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

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Robert Haas
On Nov 12, 2010, at 2:20 PM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Nov 12, 2010 at 1:55 PM, Tom Lane 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 r

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 i

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Tom Lane
Robert Haas writes: > On Fri, Nov 12, 2010 at 1:55 PM, Tom Lane 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.  I'd say leave it off the >> TODO li

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Robert Haas
On Fri, Nov 12, 2010 at 1:55 PM, Tom Lane wrote: > Andrew Dunstan 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

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Tom Lane
Andrew Dunstan 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-add it

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 an

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Bruce Momjian
Tom Lane wrote: > Andrew Dunstan 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

Re: [HACKERS] WIP: extensible enums

2010-11-12 Thread Bruce Momjian
Tom Lane wrote: > Andrew Dunstan 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

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Andrew Dunstan
On 10/24/2010 09:20 PM, Tom Lane wrote: Andrew Dunstan 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 se

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Tom Lane
Andrew Dunstan 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 >> attached. Th

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 compariso

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Tom Lane
Andrew Dunstan 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 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 The table "my

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 subscription

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 and

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 s

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Andrew Dunstan
On 10/24/2010 03:33 PM, Tom Lane wrote: Dean Rasheed 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

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Tom Lane
Dean Rasheed 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 enum_sort structs {oid, idx

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Dean Rasheed
On 24 October 2010 17:20, Tom Lane wrote: > Greg Stark 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-primiti

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 wer

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Tom Lane
Greg Stark 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 values does not s

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Dean Rasheed
On 24 October 2010 05:26, Tom Lane 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 it, but that s

Re: [HACKERS] WIP: extensible enums

2010-10-24 Thread Greg Stark
On Sat, Oct 23, 2010 at 10:11 PM, Robert Haas 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 contents of pg_enum > s

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Robert Haas
On Sun, Oct 24, 2010 at 12:26 AM, Tom Lane wrote: > Robert Haas 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, th

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Robert Haas 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 the world is to loa

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 Robert Haas
On Sat, Oct 23, 2010 at 8:11 PM, Tom Lane wrote: > Robert Haas 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

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Andrew Dunstan 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 with plain SQL, as

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Andrew Dunstan
On 10/23/2010 07:12 PM, Tom Lane wrote: Andrew Dunstan 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 between neighbours,

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Robert Haas 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 block, especially no

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Robert Haas
On Oct 23, 2010, at 7:52 PM, Tom Lane 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 of a better optio

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Robert Haas writes: > On Oct 23, 2010, at 7:12 PM, Tom Lane 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 sort ordering. Namely, that >> it d

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Robert Haas
On Oct 23, 2010, at 7:12 PM, Tom Lane wrote: > Andrew Dunstan 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 or

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Josh Berkus 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 ENUM wit

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

Re: [HACKERS] WIP: extensible enums

2010-10-23 Thread Tom Lane
Andrew Dunstan 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 reassigning the enu

Re: [HACKERS] WIP: extensible enums

2010-10-20 Thread Robert Haas
On Wed, Oct 20, 2010 at 6:54 PM, Merlin Moncure wrote: > On Tue, Oct 19, 2010 at 9:15 PM, Andrew Dunstan 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

Re: [HACKERS] WIP: extensible enums

2010-10-20 Thread Merlin Moncure
On Tue, Oct 19, 2010 at 9:15 PM, Andrew Dunstan 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 allocate enums with

Re: [HACKERS] WIP: extensible enums

2010-10-20 Thread Robert Haas
On Wed, Oct 20, 2010 at 3:16 PM, David Fetter 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 wrote: >> > Well a bit more testing shows some benefit. I've sorted out a few kinks, so >> > this seems to work. In particular, wi

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

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 Dunstan 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 create

Re: [HACKERS] WIP: extensible enums

2010-10-19 Thread Robert Haas
On Tue, Oct 19, 2010 at 5:42 PM, Andrew Dunstan 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 as > on the fresh

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 t

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 "mast

Re: [HACKERS] WIP: extensible enums

2010-10-19 Thread Magnus Hagander
On Tue, Oct 19, 2010 at 10:19, Thom Brown wrote: > On 19 October 2010 05:21, 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, >>

Re: [HACKERS] WIP: extensible enums

2010-10-19 Thread Thom Brown
On 19 October 2010 05:21, 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 compa

Re: [HACKERS] WIP: extensible enums

2010-10-19 Thread Dean Rasheed
On 19 October 2010 05:21, 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 compa

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Tom Lane
Andrew Dunstan 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 range of table sizes

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

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Tom Lane
Dean Rasheed 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 suggesting --- I'm *already* as

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Dean Rasheed
On 18 October 2010 15:52, Tom Lane 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 so, just co

Re: [HACKERS] WIP: extensible enums

2010-10-18 Thread Tom Lane
Andrew Dunstan 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 >> probe if it w

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 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 require slow c

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 don't

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Greg Stark
On Sun, Oct 17, 2010 at 12:17 PM, Tom Lane 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 value.  Prob

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Andrew Dunstan
On 10/17/2010 03:56 PM, Tom Lane wrote: Andrew Dunstan 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 seeme

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Andrew Dunstan 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 >> entry for it.

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Andrew Dunstan 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 >> pretty ug

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Andrew Dunstan 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 well enough. The

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Andrew Dunstan
On 10/17/2010 03:17 PM, Tom Lane wrote: Dean Rasheed writes: On 17 October 2010 18:53, Tom Lane 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 n

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

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Dean Rasheed writes: > On 17 October 2010 18:53, Tom Lane 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 notice that it's not >> in the cache

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 con

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Dean Rasheed
On 17 October 2010 18:53, Tom Lane 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 committed an ALTER

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 Tom Lane
Dean Rasheed writes: > On 17 October 2010 16:49, Tom Lane 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 here. > Hmm. That's coming from a new

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Dean Rasheed
On 17 October 2010 16:49, Tom Lane wrote: > Dean Rasheed 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 o

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Dean Rasheed 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 first time it was call

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Andrew Dunstan
On 10/17/2010 10:38 AM, Tom Lane wrote: Andrew Dunstan 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 n

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Dean Rasheed
On 17 October 2010 15:38, Tom Lane wrote: > Andrew Dunstan 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, bu

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Tom Lane
Andrew Dunstan 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. >> ... >> Of c

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 Rasheed 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 co

Re: [HACKERS] WIP: extensible enums

2010-10-17 Thread Dean Rasheed
On 16 October 2010 18:25, Dean Rasheed 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 that the number of

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 additiona

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 pa

Re: [HACKERS] WIP: extensible enums

2010-10-14 Thread Robert Haas
On Wed, Oct 13, 2010 at 7:33 AM, Andrew Dunstan 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 record in the CF app

Re: [HACKERS] WIP: extensible enums

2010-10-14 Thread Dean Rasheed
On 13 October 2010 23:17, Robert Haas wrote: > On Wed, Oct 13, 2010 at 7:33 AM, Andrew Dunstan 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 th

Re: [HACKERS] WIP: extensible enums

2010-10-12 Thread Robert Haas
On Fri, Oct 1, 2010 at 7:12 AM, Andrew Dunstan 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 that function

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 t

Re: [HACKERS] WIP: extensible enums

2010-10-01 Thread Dean Rasheed
On 29 September 2010 20:46, Andrew Dunstan 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 still has no do

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 e

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 Berkus writes: > I really don't see the value in making a command substantially less > intuitive in order to avoid a single keyword, unle

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 Berkus 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 outside of this part

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Tom Lane
Bruce Momjian writes: > I also noticed you grab an oid for the new type using the oid counter > without trying to make it in sorted order. Seems that would be possible > for adding enums to the end of the list, and might not be worth it. A > quick hack might be to just try of an oid+1 from the l

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Tom Lane
"David E. Wheeler" writes: > On Aug 23, 2010, at 12:20 PM, Tom Lane wrote: >> It's the three variants to do two things that I find unintuitive. > I strongly suspect that you are in the minority on this one. Yeah, seems like I'm losing the argument. Oh well. regards, tom

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Bruce Momjian
Andrew Dunstan 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 correctly sorted by oid value, and if so us

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Bruce Momjian
Josh Berkus wrote: > On 8/23/10 12:20 PM, Tom Lane wrote: > > Josh Berkus 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 outside of this particular command. > > > > It's t

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Josh Berkus
On 8/23/10 12:20 PM, Tom Lane wrote: > Josh Berkus 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 outside of this particular command. > > It's the three variants to do two thing

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread David E. Wheeler
On Aug 23, 2010, at 12:20 PM, Tom Lane wrote: > Josh Berkus 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 outside of this particular command. > > It's the three variants to do

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Andrew Dunstan
On Mon, August 23, 2010 3:20 pm, Heikki Linnakangas wrote: > On 23/08/10 22:06, Tom Lane wrote: >> Thom Brown writes: >>> On 23 August 2010 19:25, Joseph Adams >>> wrote: But what if you want to insert an OID at the end? >> >>> ALTER TYPE colors ADD 'orange'; >> >> Alternatively, if people ar

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Robert Haas
On Mon, Aug 23, 2010 at 3:06 PM, Tom Lane wrote: > Thom Brown writes: >> On 23 August 2010 19:25, Joseph Adams wrote: >>> But what if you want to insert an OID at the end? > >> ALTER TYPE colors ADD 'orange'; > > Alternatively, if people are dead set on symmetry, what we should do > to simplify

Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Tom Lane
Josh Berkus 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 outside of this particular command. It's the three variants to do two things that I find unintuitive. As I mentioned a m

  1   2   >