Re: [HACKERS] revised hstore patch
Accidentally left the doc patch out of the hstore patch posted previously, so here it is as a separate patch. -- Andrew (irc:RhodiumToad) hstore-doc-20090914.patch.gz Description: hstore doc patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
> "Ron" == Ron Mayer writes: >> At this point it's been 12 days since this was written and no >> updated patch has been posted, so I think it's well past time to >> move this to "Returned with Feedback". Accordingly I'm going to >> make that change. Hopefully, an updated patch will be ready in >> time for the September CommitFest. Ron> Curious if this patch is likely for 8.5 and/or if there's a Ron> newer patch available. I've come across an application that it Ron> seems well suited for, and would be happy to test whichever Ron> version of the patch would be most useful for me to test Ron> against. I'm planning to submit another version soon. -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Ron Mayer wrote: > Robert Haas wrote: > > On Wed, Jul 22, 2009 at 2:17 PM, Andrew > > Gierth wrote: > >> Unless I hear any objections I will proceed accordingly... > > > > At this point it's been 12 days since this was written and no updated > > patch has been posted, so I think it's well past time to move this to > > "Returned with Feedback". Accordingly I'm going to make that change. > > Hopefully, an updated patch will be ready in time for the September > > CommitFest. > > Curious if this patch is likely for 8.5 and/or if there's a newer > patch available. I've come across an application that it seems > well suited for, and would be happy to test whichever version > of the patch would be most useful for me to test against. Yea, I was wondering about this too. I do think we should just change the format and pg_migrator will detect the change and prevent migration for those cases. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Robert Haas wrote: > On Wed, Jul 22, 2009 at 2:17 PM, Andrew > Gierth wrote: >> Unless I hear any objections I will proceed accordingly... > > At this point it's been 12 days since this was written and no updated > patch has been posted, so I think it's well past time to move this to > "Returned with Feedback". Accordingly I'm going to make that change. > Hopefully, an updated patch will be ready in time for the September > CommitFest. Curious if this patch is likely for 8.5 and/or if there's a newer patch available. I've come across an application that it seems well suited for, and would be happy to test whichever version of the patch would be most useful for me to test against. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Andrew Dunstan wrote: > > > Bruce Momjian wrote: > > Tom Lane wrote: > > > >> Perhaps an appropriate thing to do is separate out the representation > >> change from the other new features, and apply just the latter for now. > >> Or maybe we should think about having two versions of hstore. This > >> is all tied up in the problem of having a decent module infrastructure > >> (which I hope somebody is working on for 8.5). I don't know where > >> we're going to end up for 8.5, but I'm disinclined to let a fairly > >> minor contrib feature improvement break upgrade-compatibility before > >> we've even really started the cycle. > >> > > > > I can just have pg_migrator detect hstore and require it be removed > > before upgrading; we did that already for 8.3 to 8.4 and I am assuming > > we will continue to have cases there pg_migrator just will not work. > > > > > > The more things you exclude the less useful the tool will be. I'm > already fairly sure it will be unusable for all or almost all my clients > who use 8.3. Sorry to hear that. You have studied the existing limitations in the README, right? I think it is important to report cases where pg_migrator doesn't work, but I don't think we will ever avoid such cases. We can't stop Postgres from moving forward, so my bet is we are always going to have such cases where pg_migrator doesn't work. I can't imagine losing a huge percentage of pg_migrator users via hstore changes, especially since you can migrate hstore manually and still use pg_migrator. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Bruce Momjian wrote: > Andrew Dunstan wrote: > > > I can't imagine losing a huge percentage of pg_migrator users via hstore > > > changes, especially since you can migrate hstore manually and still use > > > pg_migrator. > > > > > > > > > > We could finesse the hstore issues, but we are already blown out of the > > water right now by the enum issue. My biggest end client has been > > replacing small lookup tables with enums ever since they migrated to 8.3 > > many months ago. Another end client is currently moving to implement FTS > > Ah, yea, enum is an issue. > > > on 8.4, and they will be hit by the tsvector/GIN restrictions in the > > future unless we fix that. All I was saying is that the more such > > FTS will be fine for migration from 8.4 to 8.5; it was only the > internal format change that caused FTS migration not to work from 8.3 to > 8.4 (index rebuild required). There is nothing about FTS that prevents > migration. > > Here is the pg_migrator README with the restrictions: > > > http://cvs.pgfoundry.org/cgi-bin/cvsweb.cgi/pg-migrator/pg_migrator/README?rev=1.56&content-type=text/x-cvsweb-markup > > I will need to document that many of these are 8.3-8.4-only migration > issues. Done: http://cvs.pgfoundry.org/cgi-bin/cvsweb.cgi/pg-migrator/pg_migrator/README?rev=1.57&content-type=text/x-cvsweb-markup -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Andrew Dunstan wrote: > > I can't imagine losing a huge percentage of pg_migrator users via hstore > > changes, especially since you can migrate hstore manually and still use > > pg_migrator. > > > > > > We could finesse the hstore issues, but we are already blown out of the > water right now by the enum issue. My biggest end client has been > replacing small lookup tables with enums ever since they migrated to 8.3 > many months ago. Another end client is currently moving to implement FTS Ah, yea, enum is an issue. > on 8.4, and they will be hit by the tsvector/GIN restrictions in the > future unless we fix that. All I was saying is that the more such FTS will be fine for migration from 8.4 to 8.5; it was only the internal format change that caused FTS migration not to work from 8.3 to 8.4 (index rebuild required). There is nothing about FTS that prevents migration. Here is the pg_migrator README with the restrictions: http://cvs.pgfoundry.org/cgi-bin/cvsweb.cgi/pg-migrator/pg_migrator/README?rev=1.56&content-type=text/x-cvsweb-markup I will need to document that many of these are 8.3-8.4-only migration issues. > restrictions there are the less people will be able to use the tool. > Surely that is undeniable. I think it's great we (i.e. you) have made a > start on this, but there is a long way to go. Yes, I just don't want pg_migrator to be seen as a "complainer" and something that is always a drag on progress. Even if we had no data format change, using pg_migrator is still a 14-step process, so my guess is that only people with large databases where dump/reload is unreasonably long will use it, and in such cases, having to manually migrate some items is probably acceptable. What is important for me is that when pg_migrator succeeds, it is reliable, and when it can't migrate something, it is clear. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Bruce Momjian wrote: I can just have pg_migrator detect hstore and require it be removed before upgrading; we did that already for 8.3 to 8.4 and I am assuming we will continue to have cases there pg_migrator just will not work. The more things you exclude the less useful the tool will be. I'm already fairly sure it will be unusable for all or almost all my clients who use 8.3. Sorry to hear that. You have studied the existing limitations in the README, right? I think it is important to report cases where pg_migrator doesn't work, but I don't think we will ever avoid such cases. We can't stop Postgres from moving forward, so my bet is we are always going to have such cases where pg_migrator doesn't work. I can't imagine losing a huge percentage of pg_migrator users via hstore changes, especially since you can migrate hstore manually and still use pg_migrator. We could finesse the hstore issues, but we are already blown out of the water right now by the enum issue. My biggest end client has been replacing small lookup tables with enums ever since they migrated to 8.3 many months ago. Another end client is currently moving to implement FTS on 8.4, and they will be hit by the tsvector/GIN restrictions in the future unless we fix that. All I was saying is that the more such restrictions there are the less people will be able to use the tool. Surely that is undeniable. I think it's great we (i.e. you) have made a start on this, but there is a long way to go. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Bruce Momjian wrote: Tom Lane wrote: Perhaps an appropriate thing to do is separate out the representation change from the other new features, and apply just the latter for now. Or maybe we should think about having two versions of hstore. This is all tied up in the problem of having a decent module infrastructure (which I hope somebody is working on for 8.5). I don't know where we're going to end up for 8.5, but I'm disinclined to let a fairly minor contrib feature improvement break upgrade-compatibility before we've even really started the cycle. I can just have pg_migrator detect hstore and require it be removed before upgrading; we did that already for 8.3 to 8.4 and I am assuming we will continue to have cases there pg_migrator just will not work. The more things you exclude the less useful the tool will be. I'm already fairly sure it will be unusable for all or almost all my clients who use 8.3. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Tom Lane wrote: > Perhaps an appropriate thing to do is separate out the representation > change from the other new features, and apply just the latter for now. > Or maybe we should think about having two versions of hstore. This > is all tied up in the problem of having a decent module infrastructure > (which I hope somebody is working on for 8.5). I don't know where > we're going to end up for 8.5, but I'm disinclined to let a fairly > minor contrib feature improvement break upgrade-compatibility before > we've even really started the cycle. I can just have pg_migrator detect hstore and require it be removed before upgrading; we did that already for 8.3 to 8.4 and I am assuming we will continue to have cases there pg_migrator just will not work. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
On Wed, Jul 22, 2009 at 2:17 PM, Andrew Gierth wrote: > Unless I hear any objections I will proceed accordingly... At this point it's been 12 days since this was written and no updated patch has been posted, so I think it's well past time to move this to "Returned with Feedback". Accordingly I'm going to make that change. Hopefully, an updated patch will be ready in time for the September CommitFest. Thanks, ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
On Jul 22, 2009, at 11:17 AM, Andrew Gierth wrote: To me (A) is looking like the obvious choice (the people smart enough to be using hstore-new from CVS already can handle the minor pain of updating the on-disk format). Unless I hear any objections I will proceed accordingly... Yes, that seems like the smarter path to me, too, as long as the new format does not continue the bug, of course. But should the "bug" be fixed in maintenance branches? I'm thinking, since its likelihood is so rare, probably not. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
> "David" == "David E Wheeler" writes: >> The other option would be to fix the wasted-space bug in the >> existing hstore, and document that stored data must be updated at >> least once subsequent to that fix before doing a binary migrate to >> 8.5. [...] David> Could it be that only folks with such a manifestation of the David> bug would need to do a binary upgrade? If so, I certainly David> think it'd be worth it to fix the bug. Let's go through the options. A) - don't fix the wasted-space bug (or don't rely on it, anyway) - change the new format to be more distinguishable Result: - seamless binary upgrade for contrib/hstore users - users of unreleased CVS hstore-new will have to ensure all values are updated after installing a release version and before doing a binary upgrade to 8.5 B) - fix the wasted-space bug - leave the new format as-is Result: - seamless binary upgrade for hstore-new users - contrib/ users will have to remove wasted space from at least any hstore with a zero-length key before doing a binary upgrade To me (A) is looking like the obvious choice (the people smart enough to be using hstore-new from CVS already can handle the minor pain of updating the on-disk format). Unless I hear any objections I will proceed accordingly... -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
On Wed, Jul 22, 2009 at 1:40 PM, Dimitri Fontaine wrote: > Hi, > > Le 22 juil. 09 à 02:56, Robert Haas a écrit : >> >> On Tue, Jul 21, 2009 at 7:25 PM, Tom Lane wrote: >>> >>> Or maybe we should think about having two versions of hstore. This >>> is all tied up in the problem of having a decent module infrastructure >>> (which I hope somebody is working on for 8.5). > > I indeed still intend to provide some patch in the 8.5 cycle. While the user > design issue didn't receive any push back, some big items remain to be > solved. So here's my current TODO about it: > > - get some more familiar and involved in backend code by being one of the > RRR > - consider the proposed syntax ok for a first stab at it > - make the pg_catalog.pg_extension entry and the associated commands > with version as text, one thing at a time please > - bootstrap core components in pg_extension for dependancy on them > (plpgsql, ...) > - implement a backend function > pg_execute_commands_from_file('path/to/file.sql'); > reserved to superuser, file in usual accepted places > - implement INSTALL EXTENSION with the previous function > - adding a static backend local variable installing_extension (oid) > - modifying each SQL object create statement to add entries in pg_depend > - add an specific type for handling version numbers and operators comparing > them > > Here are from memory the problems we don't have a solution for yet: > - how to give user the ability to install the extension's objects in > another schema than the pg_extension default one > - how to provide extension author a way to have major PG version dependant > code without having to implement and maintain a specific function in their > install.sql file > > Please go comment on this other thread if you think the syntax is awful or > for helping me through the big tickets: > http://archives.postgresql.org/pgsql-hackers/2009-06/msg01281.php > >> A decent module infrastructure is probably not going to fix this >> problem unless it links with -ldwiw. There are really only two >> options here: > > I beg to defer. The way for a decent *extension* facility to handle the case > is by providing an upgrade function which accepts too arguments: old and new > version of the module. Then the module author is able to run custom code > from within the module upgrade transaction, where migrating on disk data > representation is entirely possible. pg_depend would have to allow for easy > finding of a given datatype column I guess. > >> (I am also not aware that anyone is working on the module >> infrastructure problem, though of course that doesn't mean that no-one >> is; but the point is that's neither here nor there as respects the >> present problem. The module infrastructure is just a management layer >> around the same underlying issues.) > > Of course if anyone wants to join, I'd appreciate. Some have offered help > and I've been failing to provide them with my todo list... but getting a > first patch for next commit fest is a goal. This would have been a good time to start a new thread with a different subject line. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Hi, Le 22 juil. 09 à 02:56, Robert Haas a écrit : On Tue, Jul 21, 2009 at 7:25 PM, Tom Lane wrote: Or maybe we should think about having two versions of hstore. This is all tied up in the problem of having a decent module infrastructure (which I hope somebody is working on for 8.5). I indeed still intend to provide some patch in the 8.5 cycle. While the user design issue didn't receive any push back, some big items remain to be solved. So here's my current TODO about it: - get some more familiar and involved in backend code by being one of the RRR - consider the proposed syntax ok for a first stab at it - make the pg_catalog.pg_extension entry and the associated commands with version as text, one thing at a time please - bootstrap core components in pg_extension for dependancy on them (plpgsql, ...) - implement a backend function pg_execute_commands_from_file('path/ to/file.sql'); reserved to superuser, file in usual accepted places - implement INSTALL EXTENSION with the previous function - adding a static backend local variable installing_extension (oid) - modifying each SQL object create statement to add entries in pg_depend - add an specific type for handling version numbers and operators comparing them Here are from memory the problems we don't have a solution for yet: - how to give user the ability to install the extension's objects in another schema than the pg_extension default one - how to provide extension author a way to have major PG version dependant code without having to implement and maintain a specific function in their install.sql file Please go comment on this other thread if you think the syntax is awful or for helping me through the big tickets: http://archives.postgresql.org/pgsql-hackers/2009-06/msg01281.php A decent module infrastructure is probably not going to fix this problem unless it links with -ldwiw. There are really only two options here: I beg to defer. The way for a decent *extension* facility to handle the case is by providing an upgrade function which accepts too arguments: old and new version of the module. Then the module author is able to run custom code from within the module upgrade transaction, where migrating on disk data representation is entirely possible. pg_depend would have to allow for easy finding of a given datatype column I guess. (I am also not aware that anyone is working on the module infrastructure problem, though of course that doesn't mean that no-one is; but the point is that's neither here nor there as respects the present problem. The module infrastructure is just a management layer around the same underlying issues.) Of course if anyone wants to join, I'd appreciate. Some have offered help and I've been failing to provide them with my todo list... but getting a first patch for next commit fest is a goal. Regards, -- dim -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
On Jul 22, 2009, at 8:55 AM, Andrew Gierth wrote: The other option would be to fix the wasted-space bug in the existing hstore, and document that stored data must be updated at least once subsequent to that fix before doing a binary migrate to 8.5. Given this… (The pathological case is _very_ rare, requiring an 0-length key with exactly 32kb of data, followed by a specific series of key lengths with values of length 1, and with more than 28kbytes of wasted space at the end of the value, and only on little-endian machines. The only way to get that much wasted space is to do several thousand iterations of UPDATE ... SET hs = hs || ''; each of which adds 8 bytes of wasted space to the end of the value. But even so, somebody, somewhere, probably has a value that matches...) Could it be that only folks with such a manifestation of the bug would need to do a binary upgrade? If so, I certainly think it'd be worth it to fix the bug. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
> "Tom" == Tom Lane writes: >> I'm prepared to give slightly more consideration to option #3: >> make the new code read the old format as well as the new one. Tom> If you think you can make that work, it would solve the problem. I was hoping to do it without changing the new format, but that turns out not to be achievable, thanks to the fact (which I just discovered) that the old hstore can leave unlimited amounts of wasted space at the end of the object (does this count as a bug?). It's doable via a small change to the new format of course. This would require some care in handling the (few) users of pgfoundry hstore-new, but all that means is that users of the pre-release hstore-new would have to ensure at least one update of stored data before doing a binary migrate to 8.5, which I think isn't too much of a burden. The other option would be to fix the wasted-space bug in the existing hstore, and document that stored data must be updated at least once subsequent to that fix before doing a binary migrate to 8.5. (The pathological case is _very_ rare, requiring an 0-length key with exactly 32kb of data, followed by a specific series of key lengths with values of length 1, and with more than 28kbytes of wasted space at the end of the value, and only on little-endian machines. The only way to get that much wasted space is to do several thousand iterations of UPDATE ... SET hs = hs || ''; each of which adds 8 bytes of wasted space to the end of the value. But even so, somebody, somewhere, probably has a value that matches...) -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Andrew Gierth wrote: The code already has users who are using it for audit-trail stuff (easily computing the changes between old and new records and storing only the differences). Perhaps one of the existing users could express an opinion on this point. I use it for exactly that purpose (and it works extremely well). I'm not sure we have any values > 64k, though, and certainly our keys are tiny - they are all column names. OTOH, that could well be an annoying limitation, and would be easily breached if the changed field were some binary object like an image or a PDF. I rather like your idea of doing a convert-on-write, if you can reliably detect that the data is in the old or new format. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
On Tue, Jul 21, 2009 at 7:25 PM, Tom Lane wrote: > Or maybe we should think about having two versions of hstore. This > is all tied up in the problem of having a decent module infrastructure > (which I hope somebody is working on for 8.5). A decent module infrastructure is probably not going to fix this problem unless it links with -ldwiw. There are really only two options here: - Keep the old version around for compatibility and add a new version that isn't compatible, plus provide a migration path from the old version to the new version. - Make the new version read the format written by the old version. (I am also not aware that anyone is working on the module infrastructure problem, though of course that doesn't mean that no-one is; but the point is that's neither here nor there as respects the present problem. The module infrastructure is just a management layer around the same underlying issues.) ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
* Andrew Gierth (and...@tao11.riddles.org.uk) wrote: > Running ALTER TABLE foo ALTER COLUMN bar TYPE hstore USING bar || ''; > on all of your hstore columns would suffice to ensure that, I believe. > (Subject to testing once I actually have code for it, of course.) This could/would be included in pg_migrator then, I would think.. Stephen signature.asc Description: Digital signature
Re: [HACKERS] revised hstore patch
> "Jeff" == Jeff Davis writes: >> I'm prepared to give slightly more consideration to option #3: >> make the new code read the old format as well as the new one. I >> believe (though I have not yet tested) that it is possible to >> reliably distinguish the two with relatively low overhead, though >> the overhead would be nonzero, and do an in-core format conversion >> (which would result in writing out the new format if anything >> changed). Jeff> It might be good to have a way to ensure that all values have Jeff> been upgraded to the new format. Otherwise you have to Jeff> permanently maintain the old format even across multiple Jeff> upgrades. Maybe that's not a big deal, but I thought I'd bring Jeff> it up. Running ALTER TABLE foo ALTER COLUMN bar TYPE hstore USING bar || ''; on all of your hstore columns would suffice to ensure that, I believe. (Subject to testing once I actually have code for it, of course.) -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
> "Tom" == Tom Lane writes: >> (b) many of the old names are significant collision risks. Tom> What collision risks? PG does not allow loadable libraries to Tom> export their symbols, so I don't see the problem. I recommend Tom> just keeping those things named as they were. You've never tested this, I can tell. I specifically checked this point, back when working on the original proposal (and when debugging the uuid code on freebsd, where uuid-ossp crashes due to a symbol collision). If a loaded module compiled from multiple source files defines some symbol, and another similar loaded module tries to define the same symbol, then whichever one gets loaded second will end up referring to the one from the first, obviously causing hilarity to ensue. I have a test case that demonstrates this and everything: % bin/psql -c 'select foo()' postgres NOTICE: mod1a foo() called NOTICE: mod1b bar() called foo - (1 row) % bin/psql -c 'select baz()' postgres NOTICE: mod2a baz() called NOTICE: mod2b bar() called baz - (1 row) % bin/psql -c 'select baz(),foo()' postgres NOTICE: mod2a baz() called NOTICE: mod2b bar() called NOTICE: mod1a foo() called NOTICE: mod2b bar() called baz | foo -+- | (1 row) % bin/psql -c 'select foo(),baz()' postgres NOTICE: mod1a foo() called NOTICE: mod1b bar() called NOTICE: mod2a baz() called NOTICE: mod1b bar() called foo | baz -+- | (1 row) Notice that in the third case, foo() called the bar() function in mod2b, not the one in mod1b which it called in the first case. All modules are compiled with pgxs and no special options. >> Certainly when developing this I had _SIGNIFICANT_ encouragement, some >> of it from YOU, for increasing the limit. Tom> Yes, but that was before the interest in in-place upgrade went up. Tom> This patch is the first place where we have to decide whether we meant Tom> it when we said we were going to pay more attention to that. >> I'm prepared to give slightly more consideration to option #3: make >> the new code read the old format as well as the new one. Tom> If you think you can make that work, it would solve the problem. OK. Should I produce an additional patch, or re-do the original one? -- Andrew. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
On Wed, 2009-07-22 at 01:06 +0100, Andrew Gierth wrote: > I'm prepared to give slightly more consideration to option #3: make > the new code read the old format as well as the new one. I believe > (though I have not yet tested) that it is possible to reliably > distinguish the two with relatively low overhead, though the overhead > would be nonzero, and do an in-core format conversion (which would > result in writing out the new format if anything changed). It might be good to have a way to ensure that all values have been upgraded to the new format. Otherwise you have to permanently maintain the old format even across multiple upgrades. Maybe that's not a big deal, but I thought I'd bring it up. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Andrew Gierth writes: > (b) many of the old names are significant collision risks. What collision risks? PG does not allow loadable libraries to export their symbols, so I don't see the problem. I recommend just keeping those things named as they were. > Certainly when developing this I had _SIGNIFICANT_ encouragement, some > of it from YOU, for increasing the limit. Yes, but that was before the interest in in-place upgrade went up. This patch is the first place where we have to decide whether we meant it when we said we were going to pay more attention to that. > I'm prepared to give slightly more consideration to option #3: make > the new code read the old format as well as the new one. If you think you can make that work, it would solve the problem. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
> "Tom" == Tom Lane writes: Tom> Andrew Gierth writes: >> Revision to previous hstore patch to fix (and add tests for) some edge >> case bugs with nulls or empty arrays. Tom> I took a quick look at this, and have a couple of beefs Tom> associated with upgrade risks. Tom> 1. The patch arbitrarily changes the C-code names of several Tom> existing SQL functions. (a) As written, it provides all of the old names too. (b) many of the old names are significant collision risks. (This was all discussed previously. I specifically said that compatibility was being maintained on this point; you obviously missed that.) Tom> 2. The patch changes the on-disk representation of hstore. That Tom> is clearly necessary to achieve the goal of allowing keys/values Tom> longer than 64K, but it breaks on-disk compatibility from 8.4 to Tom> 8.5. I'm not sure what our threshold is for allowing Tom> compatibility breaks, but I think it's higher than this. The Tom> demand for longer values inside an hstore has not been very Tom> great. The intention is that hstore(record) should work for all practically useful record sizes. While it's possible for records to be much larger than 1GB, in practice you're going to run into issues long before then. Conversely, text fields over 64k are much more common. The code already has users who are using it for audit-trail stuff (easily computing the changes between old and new records and storing only the differences). Perhaps one of the existing users could express an opinion on this point. Certainly when developing this I had _SIGNIFICANT_ encouragement, some of it from YOU, for increasing the limit. (see for example http://archives.postgresql.org/pgsql-hackers/2009-03/msg00577.php or http://archives.postgresql.org/pgsql-hackers/2009-03/msg00607.php in which alternative limits are discussed; I only noticed later that it was possible to increase the limit to 1GB for both keys and values without using extra space.) Tom> Perhaps an appropriate thing to do is separate out the Tom> representation change from the other new features, and apply Tom> just the latter for now. Or maybe we should think about having Tom> two versions of hstore. Both of those options suck (and I don't believe either would suit users of the code). I'm prepared to give slightly more consideration to option #3: make the new code read the old format as well as the new one. I believe (though I have not yet tested) that it is possible to reliably distinguish the two with relatively low overhead, though the overhead would be nonzero, and do an in-core format conversion (which would result in writing out the new format if anything changed). -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] revised hstore patch
Andrew Gierth writes: > Revision to previous hstore patch to fix (and add tests for) some edge > case bugs with nulls or empty arrays. I took a quick look at this, and have a couple of beefs associated with upgrade risks. 1. The patch arbitrarily changes the C-code names of several existing SQL functions. DO NOT DO THIS. If somebody dumps an 8.4 database containing hstore, and loads it into 8.5, the result would be crashes and perhaps even exploitable security holes. The C name is part of the function's ABI and you can't just change it. It's okay if, after such a dump and reload scenario, there is functionality that's inaccessible because the necessary SQL function definitions are missing. It's not so okay if there are security holes. 2. The patch changes the on-disk representation of hstore. That is clearly necessary to achieve the goal of allowing keys/values longer than 64K, but it breaks on-disk compatibility from 8.4 to 8.5. I'm not sure what our threshold is for allowing compatibility breaks, but I think it's higher than this. The demand for longer values inside an hstore has not been very great. Perhaps an appropriate thing to do is separate out the representation change from the other new features, and apply just the latter for now. Or maybe we should think about having two versions of hstore. This is all tied up in the problem of having a decent module infrastructure (which I hope somebody is working on for 8.5). I don't know where we're going to end up for 8.5, but I'm disinclined to let a fairly minor contrib feature improvement break upgrade-compatibility before we've even really started the cycle. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers