Thread: revised hstore patch
Revision to previous hstore patch to fix (and add tests for) some edge case bugs with nulls or empty arrays. -- Andrew (irc:RhodiumToad)
Attachment
On Jul 16, 2009, at 7:17 AM, Andrew Gierth wrote:
> Revision to previous hstore patch to fix (and add tests for) some edge
> case bugs with nulls or empty arrays.
This looks great, Andrew, thanks. Here's what I did to try it out:
* I built a simple database with a table with an (old) hstore column.
* I put in some data and wrote a bit of simple SQL to test the
existing implementation, functions, etc., as documented.
* I dumped the data.
* I applied your patch, rebuilt hstore, installed the DSO, and
restarted PotgreSQL.
* I ran the hstore `make installcheck` and all tests passed.
* I dropped the test database, created a new one, and installed hstore
into it.
* I restored the dump and ran my little regression. All the behavior
was the same. The only difference was that `hstore = hstre` started to
work instead of dying -- yay!
* I then did some experimentation to make sure that all of the new
functions and operators worked as documented. They did. I attach my
fiddling for your amusement.
Notes and minor issues:
* This line in the docs:
<entry><literal>'a=>1,b=>2'::hstore ?& ARRAY['a','b']</
literal></entry>
Needs "?&" changed to "?&
* `hstore - hstore` resolves before `hstore - text`, meaning that this
failed:
SELECT 'a=>1, b =>2'::hstore - 'a' = 'b=>2';
ERROR: Unexpected end of string
LINE 1: SELECT 'a=>1, b =>2'::hstore - 'a' = 'b=>2';
But it works if I cast it:
SELECT 'a=>1, b =>2'::hstore - 'a'::text = 'b=>2';
Not sure if there's anything to be done about that.
* There are a few operators that take text or a text array as the left
operand, such as `-` and `->`, but not with `?`. This is because the `?
` operator, which returns true if an hstore has a particular key, can
have two meanings when the left operand is an array: either it has all
the keys or it has some of the keys in the array. This patch avoids
this issue by making the former `?&` and the latter `?|`. I appreciate
the distinction, but wanted to point out that it is at the price of
inconsistency vis-a-vis some other operators (that, it must be said,
don't have the three-branch logic to deal with). I think it's a good
call, though.
* Maybe it's time to kill off `@` and `~`, eh? Or could they generate
warnings for a release or something? How are operators properly
deprecated?
* The conversion between records and hstores and using hstores to
modify particular values in records is hot.
* The documentation for `populate_record()` is wrong. It's just a cut-
and-paste of `delete()`
* The NOTE about `populoate_record()` says that it takes anyelement
instead of record and just throws an error if it's not a record. I'm
sure there's a good reason for that, but maybe there's a better way?
* Your original submission say that the new version offers btree and
hash support, but the docs still mention only GIN and GIST.
* I'd like to see more examples of the new functionality in the
documentation.
I'd like to do some word-smithing to the docs, but I'm happy to wait
until it's committed and send a new patch. Otherwise, a few minor
documentation issues notwithstanding, I think that this patch is ready
for committer review and, perhaps, committing. The code looks clean
(though mainly over my head) and the functionality is top-notch.
Best,
David
Attachment
Andrew Gierth <andrew@tao11.riddles.org.uk> 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
>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: Tom> Andrew Gierth <andrew@tao11.riddles.org.uk> writes:>> Revision to previous hstore patch to fix (and add tests for) someedge>> case bugs with nulls or empty arrays. Tom> I took a quick look at this, and have a couple of beefsTom> associated with upgrade risks. Tom> 1. The patch arbitrarily changes the C-code names of severalTom> 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. ThatTom> is clearly necessary to achieve the goal of allowingkeys/valuesTom> longer than 64K, but it breaks on-disk compatibility from 8.4 toTom> 8.5. I'm not sure what ourthreshold is for allowingTom> compatibility breaks, but I think it's higher than this. TheTom> demand for longer valuesinside an hstore has not been veryTom> 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 theTom> representation change from the other new features, and applyTom>just the latter for now. Or maybe we should think about havingTom> 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)
Andrew Gierth <andrew@tao11.riddles.org.uk> 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
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
>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: >> (b) many of the old names are significant collision risks. Tom> What collision risks? PG does not allow loadable libraries toTom> export their symbols, so I don't see the problem. I recommendTom> 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() calledfoo ----- (1 row) % bin/psql -c 'select baz()' postgres NOTICE: mod2a baz() called NOTICE: mod2b bar() calledbaz ----- (1 row) % bin/psql -c 'select baz(),foo()' postgres NOTICE: mod2a baz() called NOTICE: mod2b bar() called NOTICE: mod1a foo() called NOTICE: mod2b bar() calledbaz | 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() calledfoo | 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 todecide whether we meantTom> 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 thenew 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.
>>>>> "Jeff" == Jeff Davis <pgsql@j-davis.com> writes: >> I'm prepared to give slightly more consideration to option #3:>> make the new code read the old format as well as thenew one. I>> believe (though I have not yet tested) that it is possible to>> reliably distinguish the two with relativelylow overhead, though>> the overhead would be nonzero, and do an in-core format conversion>> (which would resultin writing out the new format if anything>> changed). Jeff> It might be good to have a way to ensure that all values haveJeff> been upgraded to the new format. Otherwise you havetoJeff> permanently maintain the old format even across multipleJeff> upgrades. Maybe that's not a big deal, but I thoughtI'd bringJeff> 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)
* Andrew Gierth (andrew@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
On Tue, Jul 21, 2009 at 7:25 PM, Tom Lane<tgl@sss.pgh.pa.us> 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
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
>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: >> I'm prepared to give slightly more consideration to option #3:>> make the new code read the old format as well as thenew 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)
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
Hi,
Le 22 juil. 09 à 02:56, Robert Haas a écrit :
> On Tue, Jul 21, 2009 at 7:25 PM, Tom Lane<tgl@sss.pgh.pa.us> 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
associatedcommands with version as text, one thing at a time please - bootstrap core components in pg_extension for
dependancyon 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
statementto 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'sobjects 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
On Wed, Jul 22, 2009 at 1:40 PM, Dimitri Fontaine<dfontaine@hi-media.com> wrote:
> Hi,
>
> Le 22 juil. 09 à 02:56, Robert Haas a écrit :
>>
>> On Tue, Jul 21, 2009 at 7:25 PM, Tom Lane<tgl@sss.pgh.pa.us> 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
>>>>> "David" == "David E Wheeler" <david@kineticode.com> writes: >> The other option would be to fix the wasted-space bug in the>> existing hstore, and document that stored data must beupdated 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 theDavid> bug would need to do a binary upgrade? If so, IcertainlyDavid> 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 distinguishableResult: - seamless binary upgrade for contrib/hstore users - users of unreleased CVS hstore-new willhave to ensure all values are updated after installing a release version and before doing a binary upgradeto 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 doinga 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)
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
On Wed, Jul 22, 2009 at 2:17 PM, Andrew Gierth<andrew@tao11.riddles.org.uk> 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
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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
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
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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
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
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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Robert Haas wrote: > On Wed, Jul 22, 2009 at 2:17 PM, Andrew > Gierth<andrew@tao11.riddles.org.uk> 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.
Ron Mayer wrote: > Robert Haas wrote: > > On Wed, Jul 22, 2009 at 2:17 PM, Andrew > > Gierth<andrew@tao11.riddles.org.uk> 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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
>>>>> "Ron" == Ron Mayer <rm_pg@cheapcomplexdevices.com> 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 pasttime to>> move this to "Returned with Feedback". Accordingly I'm going to>> make that change. Hopefully, an updatedpatch will be ready in>> time for the September CommitFest. Ron> Curious if this patch is likely for 8.5 and/or if there's aRon> newer patch available. I've come across an applicationthat itRon> seems well suited for, and would be happy to test whicheverRon> version of the patch would be mostuseful for me to testRon> against. I'm planning to submit another version soon. -- Andrew (irc:RhodiumToad)