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
hstore.sql
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
