Hi, On 2013-11-12 22:35:31 +0400, Teodor Sigaev wrote: > Attatched patch adds nesting feature, types (string, boll and numeric > values), arrays and scalar to hstore type.
I took a quick peek at this: * You cannot simply catch and ignore errors by doing + PG_TRY(); + { + n = DatumGetNumeric(DirectFunctionCall3(numeric_in, CStringGetDatum(s->val), 0, -1)); + } + PG_CATCH(); + { + n = NULL; + } + PG_END_TRY(); That skips cleanup and might ignore some errors (think memory allocation failures). But why do you even want to silently ignore errors there? * Shouldn't the checks for v->size be done before filling the datastructures in makeHStoreValueArray() and makeHStoreValuePairs()? * could you make ORDER_PAIRS() a function instead of a macro? It's pretty long and there's no reason not to use a function. * You call numeric_recv via recvHStoreValue via recvHStore without checks on the input length. That seems - without having checked it in detail - a good way to read unrelated memory. Generally ISTM the input needs to be more carefully checked in the whole recv function. * There's quite some new new, completely uncommented, code. Especially in hstore_op.c. * the _PG_init you added should probably do a EmitWarningsOnPlaceholders(). * why does hstore need it's own atoi? * shouldn't all the function prototypes be marked as externs? * Lots of trailing whitespaces, quite some long lines, cuddly braces, ... * I think hstore_compat.c's header should be updated. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers