On 01/28/2014 09:38 AM, Merlin Moncure wrote:
On Mon, Jan 27, 2014 at 9:43 PM, Andrew Dunstan <and...@dunslane.net> wrote:
On 01/26/2014 05:42 PM, Andrew Dunstan wrote:
Here is the latest set of patches for nested hstore and jsonb.
Because it's so large I've broken this into two patches and compressed
them. The jsonb patch should work standalone. The nested hstore patch
depends on it.
All the jsonb functions now use the jsonb API - there is no more turning
jsonb into text and reparsing it.
At this stage I'm going to be starting cleanup on the jsonb code
(indentation, error messages, comments etc.) as well get getting up some
jsonb docs.
Here is an update of the jsonb part of this. Charges:
* there is now documentation for jsonb
* most uses of elog() in json_funcs.c are replaced by ereport().
* indentation fixes and other tidying.
No changes in functionality.
Don't have time to fire it up this morning, but a quick scan of the
patch turned up a few minor things:
* see a comment typo, line 290 'jsonn':
* line 332: 'bogus input' -- is this up to error reporting standards?
How about "value 'x' must be one of array, object, numeric, string,
bool"?
* line 357: "jsonb's key could be only a string" prefer non
possessive: jsonb keys must be a string
* line 374, 389: ditto 332
* line 513: is panic appropriate here?
* line 599: ditto
* line 730: odd phrasing in comment, also commenting on this function
is a little light
* line 807: slightly prefer 'with respect to'
* line 888: another PANIC: these maybe correct, seems odd to halt
server on corrupted datum though*
* line 1150: hm, is the jsonb internal hash structure documented?
Aside: why didn't we use standard hash table (performance maybe)?
* line 1805-6: poor phrasing. How about: "it will order and make
unique the hash keys. Otherwise we believe that pushed keys are
ordered and unique. (Don't like verbed 'unqiue').
* line 1860: "no break here: "
Looks like this review is against jsonb-5, not jsonb-6.
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
