On Sep 18, 2009, at 6:27 PM, Andrew Gierth wrote:

However, I would prefer to keep the ability to do this:

psql --set hstore_schema='myschema' -f hstore.sql dbname

The logic to do it is a bit ugly, but editing the file to set what schema to
use is even uglier...

Yes, this should perhaps be generalized into a separate patch. I completely agree that it'd be useful and desirable.

The only open question I can see is what delete(hs,$1) resolves to when $1 is an unknown-type placeholder; this is probably an incompatibility with the old version if anyone is relying on that (but I don't see why they would be).

Given your examples, I think it probably should resolve to text as it does, as deleting a single key is likely to be a common case. It should otherwise be cast.

Because "record" doesn't express the fact that the return type of
populate_record is the SAME record type as its parameter type, whereas
anyelement does.

The lack of expressivity in records is beginning to annoy me.

David> * I'd like to see more examples of the new functionality in
David> the documentation. I'd be happy to contribute them once the
David> main patch is committed.

Thanks. I'll do some (especially for populate_record, which really needs
them), but more can be added later.

Agreed. As I said, once this is committed I'll likely go over the docs and submit a patch myself.

Old values are converted to new values in core, but they can't be
changed on-disk unless something actually updates them.

Right, of course, thanks.

The overhead is possibly non-negligible for reading old values, but
old values can be promoted to new ones fairly simply (e.g. using
ALTER TABLE).

So then it's negligible for new values?

David> * Regarding the bug you found in the old hstore format (mentioned David> [here](http://archives.postgresql.org/pgsql-hackers/2009-07/msg01422.php )
David> ), has that been fixed? Is it a regression that should be
David> back-patched?

That has not been fixed.

Should it be? I realize that it's a separate issue from this patch, and maybe it's too edge-case to bother with, given that no one has complained and it obviously won't exist once your patch is applied. Right?

David>        try=# select 'a => 1, b => 2'::hstore::text[];
David>           array
David>        -----------
David>         {a,1,b,2}

David>    I'd find this especially useful in my Perl applications, as
David>    then I could easily fetch hstores as arrays and turn them
David>    into hashes in my Perl code by simply doing `my %h = @{
David>    $row->{hstore} };`. Of course, the inverse would be useful
David>    as well:

David>        try=# select ARRAY[ 'a', '1', 'b', '2']::hstore;
David>               hstore
David>        --------------------
David>         "a"=>"1", "b"=>"2"

David>    A function would work as well, failing community interest
David>    in an explicit cast.

I'm not sure I like these as casts but I'm open to opinions. Having them
as functions is certainly doable.

I think a function would be great here. A cast is something we can decide to add later, or one can add manually using the function.

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

Reply via email to