>>>>> "David" == "David E Wheeler" <da...@kineticode.com> writes:
David> * I ran the following to update the SQL functions in my simple database: David> psql -d try --set hstore_xact='--' -f hstore.sql David> The use of `--set hstore_xact='--' was on Andrew's advice David> via IRC, because otherwise the entire update takes place in David> a single transaction, and thus would fail since I already David> had the old hstore installed. By using this psql variable David> hack to disable the transactions, I was able to install David> over the old hstore. I'm more than half inclined to take this bit out again. It made sense in the context of the pgfoundry version, but it doesn't fit in so well for contrib. (It's a concept I was testing on the pgfoundry version) 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... David> Notes and minor issues: David> * `hstore - hstore` resolves before `hstore - text`, meaning David> that this failed: The '-' operator did not previously exist so this isn't a compatibility issue. BUT the delete(hstore,text) function did previously exist, however it seems to still be resolved in preference to delete(hstore,hstore) when the second arg is a bare text literal: contrib_regression=# explain verbose select delete(('a' => now()::text),'a'); QUERY PLAN ----------------------------------------------------------- Result (cost=0.00..0.02 rows=1 width=0) Output: delete(('a'::text => (now())::text), 'a'::text) (2 rows) contrib_regression=# explain verbose select delete(('a' => now()::text),'a=>1'::hstore); QUERY PLAN -------------------------------------------------------------------- Result (cost=0.00..0.02 rows=1 width=0) Output: delete(('a'::text => (now())::text), '"a"=>"1"'::hstore) (2 rows) 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). David> * Maybe it's time to kill off `...@` and `~`, eh? Or could they David> generate warnings for a release or something? How are David> operators properly deprecated? David> * The documentation for `populate_record()` is wrong. It's David> still just a cut-and-paste of `delete()` GAH. I fixed some doc issues (stray &s) but forgot that one. I'll fix it. David> * The NOTE about `populate_record()` says that it takes David> anyelement instead of record and just throws an error if it's David> not a record. I thought that C functions could take record David> arguments. Why do the extra work here? 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. David> * Your original submission say that the new version offers David> btree and hash support, but the docs still mention only GIN David> and GIST. I'll fix that. 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. David> * I think that there should be some exmples in the docs of the David> use of the backslash with and without David> standard_conforming_strings and with and without E''. That David> stuff is confusing enough, it should all be spelled out, IMHO. ugh. yeah David> * The use of the `:hstore_xact` variable hack needs to be David> documented, (or removed, see above) David> along with detailed instructions for those upgrading David> (in-place) from 8.4. You might consider documenting your David> `:hstore_default_schema` variable as well: if I understand David> correctly, it's a way to specify a schema on the command-line David> without having to edit the file, yes? Currently, there are no David> installation instructions in the documentation. ya. David> Comments David> ======== David> * So the main objection to the original patch from the July David> CommitFest, that it wasn't backwards compatible, seems to have David> been addressed, assuming that the structure currently used in David> HEAD is just like what's in 8.4, so in-place upgrade should David> work, yes? It apears that you've taken the approach, in David> hstore_compat.c, of reading both the old and the new David> formats. Does it also upgrade the old formats as it reads David> them, or only as they're updated? (I'm assuming the latter.) Old values are converted to new values in core, but they can't be changed on-disk unless something actually updates them. David> * I'm not qualified to talk about the approach taken to David> maintaining compatibility, but would like to know if it David> imposes an overhead on the use of the data type, and if so, David> how much? 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). 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. David> * Does there need to be documentation with upgrade instructions for David> hstore-new users (the pgFoundry version)? Or will you handle that via David> a new pgFoundry release? There needs to be more documentation before the pgfoundry version gets a release, yes. I'm still testing some interactions between the three versions. David> * In addition to the functions to get all the keys and all the David> values as arrays, I'd love a function (or, dare I say, a David> cast?) to get all the rows and keys in an array. Something David> like this: 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. -- 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