On 3/30/16, Dmitry Dolgov <9erthalion6@gmail.com> wrote:
> On 31 March 2016 at 05:04, Vitaly Burovoy <vitaly.burovoy@gmail.com> wrote:
>> The documentation changes still has to be fixed.
> Thanks for help. Looks like I'm not so good at text formulation. Fixed.
Never mind. I'm also not so good at it. It is still should be
rewritten by a native English speaker, but it is better to give him
good source for it.
===
I'm almost ready to mark it as "Ready for committer".
Few notes again.
1.
> a current item was pushed to parse state after JB_PATH_INSERT_BEFORE
I paid attention that the function's name 'pushJsonbValue' looks as
working with stack (push/pop), but there is no function "pop*" neither
in jsonfuncs.c nor jsonb_util.c.
That's why I wrote "it seems the logic in the code is correct" - it is
logical to insert new value if "before", then current value, then new
value if "after".
But yes, following by executing path to the "pushState" function
anyone understands "JsonbParseState" is constructed as a stack, not a
queue. So additional comment is needed why "JB_PATH_INSERT_AFTER"
check is placed before inserting curent value and
JB_PATH_INSERT_BEFORE check.
The comment can be located just before "if (op_type &
JB_PATH_INSERT_AFTER)" (line 3986) and look similar to:
/** JsonbParseState struct behaves as a stack -- see the "pushState" function,* so check for
JB_PATH_INSERT_BEFORE/JB_PATH_INSERT_AFTERin a reverse order.*/
2.
A comment for the function "setPath" is obsolete: "If newval is null"
check and "If create is true" name do not longer exist.
Since I answered so late last time I propose the next formulating to
avoid one (possible) round:
/** Do most of the heavy work for jsonb_set/jsonb_insert** If JB_PATH_DELETE bit is set in op_type, the element is to
beremoved.** If any bit mentioned in JB_PATH_CREATE_OR_INSERT is set in op_type,* we create the new value if the key or
arrayindex does not exist.** Bits JB_PATH_INSERT_BEFORE and JB_PATH_INSERT_AFTER in op_type* behave as JB_PATH_CREATE
ifnew value is inserted in JsonbObject.** All path elements before the last must already exist* whatever bits in
op_typeare set, or nothing is done.*/
===
I hope that notes are final (additional check in formulating is appreciated).
P.S.: if it is not hard for you, please rebase the patch to the
current master to avoid offset in func.sgml.
--
Best regards,
Vitaly Burovoy