Re: [HACKERS] strip nulls functions for json and jsonb
Hi I am sending a final review of this patch: 0. this patch implements null fields stripping. It does exactly what was proposed and we would to have this feature in core. It is requested feature for JSON types. 1. there is no problem with patch apply and with compilation - one warning is fixed in attachments 2. code is relative small and clean, I have no any objection 3. there is necessary regress tests and related documentation. I have no any objection - this patch is ready for commiter. Thank you for patch Regards Pavel 2014-10-26 21:57 GMT+01:00 Andrew Dunstan and...@dunslane.net: On 10/26/2014 04:22 PM, Pavel Stehule wrote: 2014-10-26 21:18 GMT+01:00 Andrew Dunstan and...@dunslane.net mailto: and...@dunslane.net: On 10/26/2014 04:14 PM, Thom Brown wrote: On 26 October 2014 20:07, Andrew Dunstan and...@dunslane.net mailto:and...@dunslane.net mailto:and...@dunslane.net mailto:and...@dunslane.net wrote: On 10/26/2014 03:50 PM, Pavel Stehule wrote: Hi I have a question, what is expected result of null strip of {a: {b: null, c, null} } ? Please remember not to top-post. The above is not legal json, so the answer would be an error. I believe Pavel means: {a: {b: null, c: null} } This is the expected result: andrew=# select json_strip_nulls('{a: {b: null, c: null} }'); json_strip_nulls -- {a:{}} (1 row) It is NOT expected that we replace an empty object with NULL (and then strip it if it's a field value of an outer level object). ok, This case should be in regress test probably Patch attached. cheers andrew commit 03dc02f601c6b9097402317e2dfa34e0f5d33ba5 Author: Pavel Stehule pavel.steh...@gooddata.com Date: Mon Oct 27 10:53:55 2014 +0100 initial diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index f59738a..e453da4 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -10716,6 +10716,19 @@ table2-mapping /programlisting /entry /row + row + entryparaliteraljson_strip_nulls(from_json json)/literal + /paraparaliteraljsonb_strip_nulls(from_json jsonb)/literal + /para/entry + entryparatypejson/type/paraparatypejsonb/type/para/entry + entry + Returns replaceablefrom_json/replaceable + with all object fields that have null values omitted. Other null values + are untouched. + /entry + entryliteraljson_strip_nulls('[{f1:1,f2:null},2,null,3]')/literal/entry + entryliteral[{f1:1},2,null,3]/literal/entry + /row /tbody /tgroup /table @@ -10752,6 +10765,16 @@ table2-mapping /para /note + note +para + If the argument to literaljson_strip_nulls/ contains duplicate + field names in any object, the result could be semantically somewhat + different, depending on the order in which they occur. This is not an + issue for literaljsonb_strip_nulls/ since jsonb values never have + duplicate object field names. +/para + /note + para See also xref linkend=functions-aggregate for the aggregate function functionjson_agg/function which aggregates record diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 2d00dbe..efbec7f 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -105,6 +105,15 @@ static void populate_recordset_object_end(void *state); static void populate_recordset_array_start(void *state); static void populate_recordset_array_element_start(void *state, bool isnull); +/* semantic action functions for json_strip_nulls */ +static void sn_object_start(void *state); +static void sn_object_end(void *state); +static void sn_array_start(void *state); +static void sn_array_end(void *state); +static void sn_object_field_start (void *state, char *fname, bool isnull); +static void sn_array_element_start (void *state, bool isnull); +static void sn_scalar(void *state, char *token, JsonTokenType tokentype); + /* worker function for populate_recordset and to_recordset */ static Datum populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname, bool have_record_arg); @@ -225,6 +234,13 @@ typedef struct PopulateRecordsetState MemoryContext fn_mcxt; /* used to stash IO funcs */ } PopulateRecordsetState; +/* state for json_strip_nulls */ +typedef struct StripnullState{ + JsonLexContext *lex; + StringInfo strval; + bool skip_next_null; +} StripnullState; + /* Turn a jsonb object into a record */ static void make_row_from_rec_and_jsonb(Jsonb *element, PopulateRecordsetState *state); @@ -2996,3 +3012,184 @@ findJsonbValueFromContainerLen(JsonbContainer *container, uint32 flags, return findJsonbValueFromContainer(container,
Re: [HACKERS] strip nulls functions for json and jsonb
On 10/16/2014 04:12 PM, Pavel Stehule wrote: 1. missing documentation 2. I miss more comments related to this functions. This code is relative simple, but some more explanation can be welcome. 3. why these functions are marked as stable? New patch: Docs added, functions marked immutable, more comments added. cheers andrew diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 7e5bcd9..352b408 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -10716,6 +10716,19 @@ table2-mapping /programlisting /entry /row + row + entryparaliteraljson_strip_nulls(from_json json)/literal + /paraparaliteraljsonb_strip_nulls(from_json jsonb)/literal + /para/entry + entryparatypejson/type/paraparatypejsonb/type/para/entry + entry + Returns replaceablefrom_json/replaceable + with all object fields that have null values omitted. Other null values + are untouched. + /entry + entryliteraljson_strip_nulls('[{f1:1,f2:null},2,null,3]')/literal/entry + entryliteral[{f1:1},2,null,3]/literal/entry + /row /tbody /tgroup /table @@ -10752,6 +10765,16 @@ table2-mapping /para /note + note +para + If the argument to literaljson_strip_nulls/ contains duplicate + field names in any object, the result could be semantically somewhat + different, depending on the order in which they occur. This is not an + issue for literaljsonb_strip_nulls/ since jsonb values never have + duplicate object field names. +/para + /note + para See also xref linkend=functions-aggregate for the aggregate function functionjson_agg/function which aggregates record diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 2d00dbe..06db3e4 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -105,6 +105,15 @@ static void populate_recordset_object_end(void *state); static void populate_recordset_array_start(void *state); static void populate_recordset_array_element_start(void *state, bool isnull); +/* semantic action functions for json_strip_nulls */ +static void sn_object_start(void *state); +static void sn_object_end(void *state); +static void sn_array_start(void *state); +static void sn_array_end(void *state); +static void sn_object_field_start (void *state, char *fname, bool isnull); +static void sn_array_element_start (void *state, bool isnull); +static void sn_scalar(void *state, char *token, JsonTokenType tokentype); + /* worker function for populate_recordset and to_recordset */ static Datum populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname, bool have_record_arg); @@ -225,6 +234,13 @@ typedef struct PopulateRecordsetState MemoryContext fn_mcxt; /* used to stash IO funcs */ } PopulateRecordsetState; +/* state for json_strip_nulls */ +typedef struct StripnullState{ + JsonLexContext *lex; + StringInfo strval; + bool skip_next_null; +} StripnullState; + /* Turn a jsonb object into a record */ static void make_row_from_rec_and_jsonb(Jsonb *element, PopulateRecordsetState *state); @@ -2996,3 +3012,184 @@ findJsonbValueFromContainerLen(JsonbContainer *container, uint32 flags, return findJsonbValueFromContainer(container, flags, k); } + +/* + * Semantic actions for json_strip_nulls. + * + * Simply repeat the input on the output unless we encounter + * a null object field. State for this is set when the field + * is started and reset when the scalar action (which must be next) + * is called. + */ + +static void +sn_object_start(void *state) +{ + StripnullState *_state = (StripnullState *) state; + appendStringInfoCharMacro(_state-strval, '{'); +} + +static void +sn_object_end(void *state) +{ + StripnullState *_state = (StripnullState *) state; + appendStringInfoCharMacro(_state-strval, '}'); +} + +static void +sn_array_start(void *state) +{ + StripnullState *_state = (StripnullState *) state; + appendStringInfoCharMacro(_state-strval, '['); +} + +static void +sn_array_end(void *state) +{ + StripnullState *_state = (StripnullState *) state; + appendStringInfoCharMacro(_state-strval, ']'); +} + +static void +sn_object_field_start (void *state, char *fname, bool isnull) +{ + StripnullState *_state = (StripnullState *) state; + + if (isnull) + { + /* + * The next thing must be a scalar or isnull couldn't be true, + * so there is no danger of this state being carried down + * into a nested object or array. The flag will be reset in the + * scalar action. + */ + _state-skip_next_null = true; + return; + } + + if (_state-strval-data[_state-strval-len - 1] != '{') + appendStringInfoCharMacro(_state-strval, ','); + + /* + * Unfortunately we don't have the quoted and escaped string any more, + * so we have to re-escape it. + */ + escape_json(_state-strval,fname); + + appendStringInfoCharMacro(_state-strval, ':'); +} + +static
Re: [HACKERS] strip nulls functions for json and jsonb
Hi I have a question, what is expected result of null strip of {a: {b: null, c, null} } ? 2014-10-26 19:59 GMT+01:00 Andrew Dunstan and...@dunslane.net: On 10/16/2014 04:12 PM, Pavel Stehule wrote: 1. missing documentation 2. I miss more comments related to this functions. This code is relative simple, but some more explanation can be welcome. 3. why these functions are marked as stable? New patch: Docs added, functions marked immutable, more comments added. cheers andrew
Re: [HACKERS] strip nulls functions for json and jsonb
On 10/26/2014 03:50 PM, Pavel Stehule wrote: Hi I have a question, what is expected result of null strip of {a: {b: null, c, null} } ? Please remember not to top-post. The above is not legal json, so the answer would be an error. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] strip nulls functions for json and jsonb
On 26 October 2014 20:07, Andrew Dunstan and...@dunslane.net wrote: On 10/26/2014 03:50 PM, Pavel Stehule wrote: Hi I have a question, what is expected result of null strip of {a: {b: null, c, null} } ? Please remember not to top-post. The above is not legal json, so the answer would be an error. I believe Pavel means: {a: {b: null, c: null} } -- Thom
Re: [HACKERS] strip nulls functions for json and jsonb
2014-10-26 21:18 GMT+01:00 Andrew Dunstan and...@dunslane.net: On 10/26/2014 04:14 PM, Thom Brown wrote: On 26 October 2014 20:07, Andrew Dunstan and...@dunslane.net mailto: and...@dunslane.net wrote: On 10/26/2014 03:50 PM, Pavel Stehule wrote: Hi I have a question, what is expected result of null strip of {a: {b: null, c, null} } ? Please remember not to top-post. The above is not legal json, so the answer would be an error. I believe Pavel means: {a: {b: null, c: null} } This is the expected result: andrew=# select json_strip_nulls('{a: {b: null, c: null} }'); json_strip_nulls -- {a:{}} (1 row) It is NOT expected that we replace an empty object with NULL (and then strip it if it's a field value of an outer level object). ok, This case should be in regress test probably Thank you Pavel cheers andrew
Re: [HACKERS] strip nulls functions for json and jsonb
On 10/26/2014 04:14 PM, Thom Brown wrote: On 26 October 2014 20:07, Andrew Dunstan and...@dunslane.net mailto:and...@dunslane.net wrote: On 10/26/2014 03:50 PM, Pavel Stehule wrote: Hi I have a question, what is expected result of null strip of {a: {b: null, c, null} } ? Please remember not to top-post. The above is not legal json, so the answer would be an error. I believe Pavel means: {a: {b: null, c: null} } This is the expected result: andrew=# select json_strip_nulls('{a: {b: null, c: null} }'); json_strip_nulls -- {a:{}} (1 row) It is NOT expected that we replace an empty object with NULL (and then strip it if it's a field value of an outer level object). cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] strip nulls functions for json and jsonb
On 10/26/2014 04:22 PM, Pavel Stehule wrote: 2014-10-26 21:18 GMT+01:00 Andrew Dunstan and...@dunslane.net mailto:and...@dunslane.net: On 10/26/2014 04:14 PM, Thom Brown wrote: On 26 October 2014 20:07, Andrew Dunstan and...@dunslane.net mailto:and...@dunslane.net mailto:and...@dunslane.net mailto:and...@dunslane.net wrote: On 10/26/2014 03:50 PM, Pavel Stehule wrote: Hi I have a question, what is expected result of null strip of {a: {b: null, c, null} } ? Please remember not to top-post. The above is not legal json, so the answer would be an error. I believe Pavel means: {a: {b: null, c: null} } This is the expected result: andrew=# select json_strip_nulls('{a: {b: null, c: null} }'); json_strip_nulls -- {a:{}} (1 row) It is NOT expected that we replace an empty object with NULL (and then strip it if it's a field value of an outer level object). ok, This case should be in regress test probably Patch attached. cheers andrew diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 7e5bcd9..352b408 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -10716,6 +10716,19 @@ table2-mapping /programlisting /entry /row + row + entryparaliteraljson_strip_nulls(from_json json)/literal + /paraparaliteraljsonb_strip_nulls(from_json jsonb)/literal + /para/entry + entryparatypejson/type/paraparatypejsonb/type/para/entry + entry + Returns replaceablefrom_json/replaceable + with all object fields that have null values omitted. Other null values + are untouched. + /entry + entryliteraljson_strip_nulls('[{f1:1,f2:null},2,null,3]')/literal/entry + entryliteral[{f1:1},2,null,3]/literal/entry + /row /tbody /tgroup /table @@ -10752,6 +10765,16 @@ table2-mapping /para /note + note +para + If the argument to literaljson_strip_nulls/ contains duplicate + field names in any object, the result could be semantically somewhat + different, depending on the order in which they occur. This is not an + issue for literaljsonb_strip_nulls/ since jsonb values never have + duplicate object field names. +/para + /note + para See also xref linkend=functions-aggregate for the aggregate function functionjson_agg/function which aggregates record diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 2d00dbe..06db3e4 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -105,6 +105,15 @@ static void populate_recordset_object_end(void *state); static void populate_recordset_array_start(void *state); static void populate_recordset_array_element_start(void *state, bool isnull); +/* semantic action functions for json_strip_nulls */ +static void sn_object_start(void *state); +static void sn_object_end(void *state); +static void sn_array_start(void *state); +static void sn_array_end(void *state); +static void sn_object_field_start (void *state, char *fname, bool isnull); +static void sn_array_element_start (void *state, bool isnull); +static void sn_scalar(void *state, char *token, JsonTokenType tokentype); + /* worker function for populate_recordset and to_recordset */ static Datum populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname, bool have_record_arg); @@ -225,6 +234,13 @@ typedef struct PopulateRecordsetState MemoryContext fn_mcxt; /* used to stash IO funcs */ } PopulateRecordsetState; +/* state for json_strip_nulls */ +typedef struct StripnullState{ + JsonLexContext *lex; + StringInfo strval; + bool skip_next_null; +} StripnullState; + /* Turn a jsonb object into a record */ static void make_row_from_rec_and_jsonb(Jsonb *element, PopulateRecordsetState *state); @@ -2996,3 +3012,184 @@ findJsonbValueFromContainerLen(JsonbContainer *container, uint32 flags, return findJsonbValueFromContainer(container, flags, k); } + +/* + * Semantic actions for json_strip_nulls. + * + * Simply repeat the input on the output unless we encounter + * a null object field. State for this is set when the field + * is started and reset when the scalar action (which must be next) + * is called. + */ + +static void +sn_object_start(void *state) +{ + StripnullState *_state = (StripnullState *) state; + appendStringInfoCharMacro(_state-strval, '{'); +} + +static void +sn_object_end(void *state) +{ + StripnullState *_state = (StripnullState *) state; + appendStringInfoCharMacro(_state-strval, '}'); +} + +static void +sn_array_start(void *state) +{ + StripnullState *_state = (StripnullState *) state; + appendStringInfoCharMacro(_state-strval, '['); +} + +static void +sn_array_end(void *state) +{
Re: [HACKERS] strip nulls functions for json and jsonb
Hello I checked this patch. 1. There is a consensus we want this feature. 2. This patch implement just this mentioned feature. There is no objection against design. 3. It is possible to apply this patch and compile without warnings. 4. I tested null stripping on large json, jsonb values without problems. 5. regress tests are enough 6. code is well formatted Objections questions: 1. missing documentation 2. I miss more comments related to this functions. This code is relative simple, but some more explanation can be welcome. 3. why these functions are marked as stable? Regards Pavel 2014-10-04 1:23 GMT+02:00 Andrew Dunstan and...@dunslane.net: As discussed recently, here is an undocumented patch for json_strip_nulls and jsonb_strip_nulls. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] strip nulls functions for json and jsonb
As discussed recently, here is an undocumented patch for json_strip_nulls and jsonb_strip_nulls. cheers andrew diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 2d00dbe..e9636d8 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -105,6 +105,15 @@ static void populate_recordset_object_end(void *state); static void populate_recordset_array_start(void *state); static void populate_recordset_array_element_start(void *state, bool isnull); +/* semantic action functions for json_strip_nulls */ +static void sn_object_start(void *state); +static void sn_object_end(void *state); +static void sn_array_start(void *state); +static void sn_array_end(void *state); +static void sn_object_field_start (void *state, char *fname, bool isnull); +static void sn_array_element_start (void *state, bool isnull); +static void sn_scalar(void *state, char *token, JsonTokenType tokentype); + /* worker function for populate_recordset and to_recordset */ static Datum populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname, bool have_record_arg); @@ -225,6 +234,13 @@ typedef struct PopulateRecordsetState MemoryContext fn_mcxt; /* used to stash IO funcs */ } PopulateRecordsetState; +/* state for json_strip_nulls */ +typedef struct StripnullState{ + JsonLexContext *lex; + StringInfo strval; + bool skip_next_null; +} StripnullState; + /* Turn a jsonb object into a record */ static void make_row_from_rec_and_jsonb(Jsonb *element, PopulateRecordsetState *state); @@ -2996,3 +3012,169 @@ findJsonbValueFromContainerLen(JsonbContainer *container, uint32 flags, return findJsonbValueFromContainer(container, flags, k); } + +static void +sn_object_start(void *state) +{ + StripnullState *_state = (StripnullState *) state; + appendStringInfoCharMacro(_state-strval, '{'); +} + +static void +sn_object_end(void *state) +{ + StripnullState *_state = (StripnullState *) state; + appendStringInfoCharMacro(_state-strval, '}'); +} + +static void +sn_array_start(void *state) +{ + StripnullState *_state = (StripnullState *) state; + appendStringInfoCharMacro(_state-strval, '['); +} + +static void +sn_array_end(void *state) +{ + StripnullState *_state = (StripnullState *) state; + appendStringInfoCharMacro(_state-strval, ']'); +} + +static void +sn_object_field_start (void *state, char *fname, bool isnull) +{ + StripnullState *_state = (StripnullState *) state; + + if (isnull) + { + _state-skip_next_null = true; + return; + } + + if (_state-strval-data[_state-strval-len - 1] != '{') + appendStringInfoCharMacro(_state-strval, ','); + + /* + * Unfortunately we don't have the quoted and escaped string any more, + * so we have to re-escape it. + */ + escape_json(_state-strval,fname); + + appendStringInfoCharMacro(_state-strval, ':'); +} + +static void +sn_array_element_start (void *state, bool isnull) +{ + StripnullState *_state = (StripnullState *) state; + + if (_state-strval-data[_state-strval-len - 1] != '[') + appendStringInfoCharMacro(_state-strval, ','); +} + +static void +sn_scalar(void *state, char *token, JsonTokenType tokentype) +{ + StripnullState *_state = (StripnullState *) state; + + if (_state-skip_next_null) + { + _state-skip_next_null = false; + return; + } + + if (tokentype == JSON_TOKEN_STRING) + escape_json(_state-strval, token); + else + appendStringInfoString(_state-strval, token); +} + +/* + * SQL function json_strip_nulls(json) - json + */ +Datum +json_strip_nulls(PG_FUNCTION_ARGS) +{ + text *json = PG_GETARG_TEXT_P(0); + StripnullState *state; + JsonLexContext *lex; + JsonSemAction *sem; + + lex = makeJsonLexContext(json, true); + state = palloc0(sizeof(StripnullState)); + sem = palloc0(sizeof(JsonSemAction)); + + state-strval = makeStringInfo(); + state-skip_next_null = false; + state-lex = lex; + + sem-semstate = (void *) state; + sem-object_start = sn_object_start; + sem-object_end = sn_object_end; + sem-array_start = sn_array_start; + sem-array_end = sn_array_end; + sem-scalar = sn_scalar; + sem-array_element_start = sn_array_element_start; + sem-object_field_start = sn_object_field_start; + + pg_parse_json(lex, sem); + + PG_RETURN_TEXT_P(cstring_to_text_with_len(state-strval-data, + state-strval-len)); + +} + +/* + * SQL function jsonb_strip_nulls(jsonb) - jsonb + */ +Datum +jsonb_strip_nulls(PG_FUNCTION_ARGS) +{ + Jsonb * jb = PG_GETARG_JSONB(0); + JsonbIterator *it; + JsonbParseState *parseState = NULL; + JsonbValue *res; + int type; + JsonbValue v,k; + bool last_was_key = false; + + if (JB_ROOT_IS_SCALAR(jb)) + PG_RETURN_POINTER(jb); + + it = JsonbIteratorInit(jb-root); + + while ((type = JsonbIteratorNext(it, v, false)) != WJB_DONE) + { + Assert( ! (type == WJB_KEY last_was_key)); + + if (type == WJB_KEY) + { + /* stash the key until we know if it has a null value */ + k = v; + last_was_key = true; + continue; + } + + if (last_was_key) + { + /* skip this
Re: [HACKERS] strip nulls functions for json and jsonb
Hi 2014-10-04 1:23 GMT+02:00 Andrew Dunstan and...@dunslane.net: As discussed recently, here is an undocumented patch for json_strip_nulls and jsonb_strip_nulls. It is looking well Regards Pavel cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers