Hi,
I looked at the patch and have several comments.
First let me say that modifying the individual paths of the json is the
feature I miss the most in the current implementation so I am happy that
this patch was submitted.
I would prefer slightly the patch split into two parts, one for the
indent printing and one with the manipulation functions, but it's not
too big patch so it's not too bad as it is.
There is one compiler warning that I see:
jsonfuncs.c:3533:1: warning: no previous prototype for
‘jsonb_delete_path’ [-Wmissing-prototypes] jsonb_delete_path(PG_FUNCTION_ARGS)
I think it would be better if the ident printing didn't put the start of
array ([) and start of dictionary ({) on separate line since most
"pretty" printing implementations outside of Postgres (like the
JSON.stringify or python's json module) don't do that. This is purely
about consistency with the rest of the world.
The json_ident might be better named as json_pretty perhaps?
I don't really understand the point of h_atoi() function. What's wrong
with using strtol like pg_atoi does? Also there is no integer overflow
check anywhere in that function.
There is tons of end of line whitespace mess in jsonb_indent docs.
Otherwise everything I tried so far works as expected. The code looks ok
as well except maybe the replacePath could use couple of comments (for
example the line which uses the h_atoi) to make it easier to follow.
About the:
> + /* XXX : why do we need this assertion? The functions is declared to take text[] */
> + Assert(ARR_ELEMTYPE(path) == TEXTOID);
I wonder about this also, some functions do that, some don't, I am not
really sure what is the rule there myself.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services