On 3/25/16, Dmitry Dolgov <9erthali...@gmail.com> wrote: > Here is a new version of path, I hope I didn't miss anything. Few notes: > >> 4. >> or even create a new constant (there can be better name for it): >> #define JB_PATH_CREATE_OR_INSERT (JB_PATH_INSERT_BEFORE | >> JB_PATH_INSERT_AFTER | JB_PATH_CREATE) > > Good idea, thanks.
You're welcome. === I'm sorry for the late letter. Unfortunately, it seems one more round is necessary. The documentation changes still has to be fixed. The main description points to a "target section designated by path is a JSONB array". It is a copy-paste from jsonb_set, but here it has wrong context. The main idea in jsonb_set is replacing any object which is pointed (designated) by "path" no matter where (array or object) it is located. But in the phrase for jsonb_insert above you want to point to an upper object _where_ "section designated by path" is located. I'd write something like "If target section designated by path is _IN_ a JSONB array, ...". The same thing with "a JSONB object". Also I'd rewrite "will act like" as "behaves as". Last time I missed the argument's name "insert_before_after". It is better to replace it as just "insert_after". Also reflect that name in the documentation properly. To be consistent change the constant "JB_PATH_NOOP" as "0x0000" instead of just "0x0". Also the variable's name "bool before" is incorrect because it contradicts with the SQL function's argument "after" (from the documentation) or "insert_after" (proposed above) or tests (de-facto behavior). Moreover it seems the logic in the code is correct, so I have no idea why "before ? JB_PATH_INSERT_BEFORE : JB_PATH_INSERT_AFTER" works. I think either proper comment should be added or lack in the code must be found. Anyway the variable's name must reflect the SQL argument's name. -- Best regards, Vitaly Burovoy -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers