On Sun, Jan 20, 2019 at 6:30 AM Alexander Korotkov <a.korot...@postgrespro.ru> wrote: > I'll continue revising this patchset. Nikita, could you please write > tests for 3-argument versions of functions? Also, please answer the > question regarding "id" property.
I've some more notes regarding function set provided in jsonpath patch: 1) Function names don't follow the convention we have. All our functions dealing with jsonb have "jsonb_" prefix. Exclusions have "jsonb" in other part of name, for example, "to_jsonb(anyelement)". We could name functions at SQL level in the same way they are named in C. So, they would be jsonb_jsonpath_exists() etc. But it's probably too long. What about jsonb_path_exists() etc? 2) jsonpath_query_wrapped() name seems counter intuitive for me. What about jsonpath_query_array()? 3) jsonpath_query_wrapped() behavior looks tricky to me. It returns NULL for no results. When result item is one, it is returned "as is". When there are two or more result items, they are wrapped into array. This representation of result seems extremely inconvenient for me. It becomes hard to solve even trivial tasks with that: for instance, iterate all items found. Without extra assumptions on what is going to be returned it's also impossible for caller to distinguish single array item found from multiple items found wrapped into array. And that seems very bad. I know this behavior is inspired by SQL/JSON standard. But since these functions are anyway our extension, we can implement them as we like. So, I propose to make this function always return array of items regardless on count of those items (even for 1 or 0 items). 4) If we change behavior of jsonpath_query_wrapped() as above, we can also implement jsonpath_query_one() function, which would return first result item or NULL for no items. Any thoughts? ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
