Re: updated hstore patch
От | Andrew Gierth |
---|---|
Тема | Re: updated hstore patch |
Дата | |
Msg-id | 87ws3vn2pe.fsf@news-spur.riddles.org.uk обсуждение исходный текст |
Ответ на | Re: updated hstore patch ("David E. Wheeler" <david@kineticode.com>) |
Ответы |
Re: updated hstore patch
(Magnus Hagander <magnus@hagander.net>)
Re: updated hstore patch ("David E. Wheeler" <david@kineticode.com>) |
Список | pgsql-hackers |
>>>>> "David" == "David E Wheeler" <david@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 adviceDavid> via IRC, because otherwise the entire updatetakes place inDavid> a single transaction, and thus would fail since I alreadyDavid> had the old hstore installed.By using this psql variableDavid> hack to disable the transactions, I was able to installDavid> over theold 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`, meaningDavid> 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); QUERYPLAN --------------------------------------------------------------------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 theyDavid> generate warnings for a release or something? HowareDavid> operators properly deprecated? David> * The documentation for `populate_record()` is wrong. It'sDavid> 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 takesDavid> anyelement instead of record and just throws an errorif it'sDavid> not a record. I thought that C functions could take recordDavid> 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 offersDavid> btree and hash support, but the docs still mentiononly GINDavid> and GIST. I'll fix that. David> * I'd like to see more examples of the new functionality inDavid> the documentation. I'd be happy to contribute themonce theDavid> 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 theDavid> use of the backslash with and withoutDavid> standard_conforming_stringsand with and without E''. ThatDavid> stuff is confusing enough, it should all be spelled out,IMHO. ugh. yeah David> * The use of the `:hstore_xact` variable hack needs to beDavid> documented, (or removed, see above) David> along with detailed instructions for those upgradingDavid> (in-place) from 8.4. You might consider documenting yourDavid>`:hstore_default_schema` variable as well: if I understandDavid> correctly, it's a way to specify a schema on thecommand-lineDavid> without having to edit the file, yes? Currently, there are noDavid> installation instructions in thedocumentation. ya. David> CommentsDavid> ======== David> * So the main objection to the original patch from the JulyDavid> CommitFest, that it wasn't backwards compatible,seems to haveDavid> been addressed, assuming that the structure currently used inDavid> HEAD is just like what'sin 8.4, so in-place upgrade shouldDavid> work, yes? It apears that you've taken the approach, inDavid> hstore_compat.c,of reading both the old and the newDavid> formats. Does it also upgrade the old formats as it readsDavid>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 toDavid> maintaining compatibility, but would like to know ifitDavid> 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 (mentionedDavid> [here](http://archives.postgresql.org/pgsql-hackers/2009-07/msg01422.php)David>), has that been fixed? Is it a regressionthat should beDavid> back-patched? That has not been fixed. David> * Does there need to be documentation with upgrade instructions forDavid> hstore-new users (the pgFoundry version)?Or will you handle that viaDavid> 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 theDavid> values as arrays, I'd love a function (or, dareI say, aDavid> cast?) to get all the rows and keys in an array. SomethingDavid> like this: David> try=# select 'a => 1, b => 2'::hstore::text[];David> arrayDavid> -----------David> {a,1,b,2} David> I'd find this especially useful in my Perl applications, asDavid> then I could easily fetch hstores as arraysand turn themDavid> into hashes in my Perl code by simply doing `my %h = @{David> $row->{hstore} };`. Of course,the inverse would be usefulDavid> as well: David> try=# select ARRAY[ 'a', '1', 'b', '2']::hstore;David> hstoreDavid> --------------------David> "a"=>"1", "b"=>"2" David> A function would work as well, failing community interestDavid> 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)
В списке pgsql-hackers по дате отправления: