Re: [HACKERS] revised hstore patch

2009-09-15 Thread Andrew Gierth
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


[HACKERS] revised hstore patch

2009-09-14 Thread Andrew Gierth
Latest hstore patch with provision for inplace upgrading.

-- 
Andrew (irc:RhodiumToad)



hstore-20090914.patch.gz
Description: hstore 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

2009-08-24 Thread Andrew Gierth
 Ron == Ron Mayer rm...@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 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

2009-08-21 Thread Ron Mayer
Robert Haas wrote:
 On Wed, Jul 22, 2009 at 2:17 PM, Andrew
 Gierthand...@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.


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

2009-08-21 Thread Bruce Momjian
Ron Mayer wrote:
 Robert Haas wrote:
  On Wed, Jul 22, 2009 at 2:17 PM, Andrew
  Gierthand...@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  br...@momjian.ushttp://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

2009-08-09 Thread Andrew Dunstan



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

2009-08-09 Thread Andrew Dunstan



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

2009-08-09 Thread Bruce Momjian
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.56content-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  br...@momjian.ushttp://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

2009-08-09 Thread Bruce Momjian
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.56content-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.57content-type=text/x-cvsweb-markup

-- 
  Bruce Momjian  br...@momjian.ushttp://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

2009-08-09 Thread Bruce Momjian
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  br...@momjian.ushttp://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

2009-08-08 Thread Bruce Momjian
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  br...@momjian.ushttp://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

2009-08-02 Thread Robert Haas
On Wed, Jul 22, 2009 at 2:17 PM, Andrew
Gierthand...@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

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

2009-07-23 Thread David E. Wheeler

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

2009-07-22 Thread Andrew Gierth
 Tom == Tom Lane t...@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 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

2009-07-22 Thread David E. Wheeler

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

2009-07-22 Thread Dimitri Fontaine

Hi,

Le 22 juil. 09 à 02:56, Robert Haas a écrit :

On Tue, Jul 21, 2009 at 7:25 PM, Tom Lanet...@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.


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

2009-07-22 Thread Robert Haas
On Wed, Jul 22, 2009 at 1:40 PM, Dimitri Fontainedfonta...@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 Lanet...@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

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

2009-07-22 Thread Andrew Gierth
 David == David E Wheeler da...@kineticode.com 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

2009-07-21 Thread Tom Lane
Andrew Gierth and...@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

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

2009-07-21 Thread Andrew Gierth
 Tom == Tom Lane t...@sss.pgh.pa.us writes:

 Tom Andrew Gierth and...@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.

 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

2009-07-21 Thread Tom Lane
Andrew Gierth and...@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

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

2009-07-21 Thread Jeff Davis
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

2009-07-21 Thread Andrew Gierth
 Tom == Tom Lane t...@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 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

2009-07-21 Thread Andrew Gierth
 Jeff == Jeff Davis pg...@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 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

2009-07-21 Thread Stephen Frost
* 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

2009-07-21 Thread Robert Haas
On Tue, Jul 21, 2009 at 7:25 PM, Tom Lanet...@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

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

2009-07-21 Thread Andrew Dunstan



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


[HACKERS] revised hstore patch

2009-07-16 Thread Andrew Gierth
Revision to previous hstore patch to fix (and add tests for) some edge
case bugs with nulls or empty arrays.

-- 
Andrew (irc:RhodiumToad)



hstore_20090716.patch.gz
Description: hstore patch

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers