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> + <entry><para><literal>json_strip_nulls(from_json json)</literal> + </para><para><literal>jsonb_strip_nulls(from_json jsonb)</literal> + </para></entry> + <entry><para><type>json</type></para><para><type>jsonb</type></para></entry> + <entry> + Returns <replaceable>from_json</replaceable> + with all object fields that have null values omitted. Other null values + are untouched. + </entry> + <entry><literal>json_strip_nulls('[{"f1":1,"f2":null},2,null,3]')</literal></entry> + <entry><literal>[{"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 <literal>json_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 <literal>jsonb_strip_nulls</> since jsonb values never have + duplicate object field names. + </para> + </note> + <para> See also <xref linkend="functions-aggregate"> for the aggregate function <function>json_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, 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 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) + { + Assert (tokentype == JSON_TOKEN_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 = NULL; + 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) + { + /* if the last element was a key this one can't be */ + last_was_key = false; + + /* skip this field if value is null */ + if (type == WJB_VALUE && v.type == jbvNull) + continue; + + /* otherwise, do a delayed push of the key */ + res = pushJsonbValue(&parseState, WJB_KEY, &k); + } + + if (type == WJB_VALUE || type == WJB_ELEM) + res = pushJsonbValue(&parseState, type, &v); + else + res = pushJsonbValue(&parseState, type, NULL); + } + + PG_RETURN_POINTER(JsonbValueToJsonb(res)); +} diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h index 9273c1f..46a645a 100644 --- a/src/include/catalog/pg_proc.h +++ b/src/include/catalog/pg_proc.h @@ -4243,6 +4243,8 @@ DATA(insert OID = 3203 ( json_object PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 DESCR("map text arrays of keys and values to json object"); DATA(insert OID = 3176 ( to_json PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 114 "2283" _null_ _null_ _null_ _null_ to_json _null_ _null_ _null_ )); DESCR("map input to json"); +DATA(insert OID = 3300 ( json_strip_nulls PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 114 "114" _null_ _null_ _null_ _null_ json_strip_nulls _null_ _null_ _null_ )); +DESCR("remove object fields with null values from json"); DATA(insert OID = 3947 ( json_object_field PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 114 "114 25" _null_ _null_ "{from_json, field_name}" _null_ json_object_field _null_ _null_ _null_ )); DATA(insert OID = 3948 ( json_object_field_text PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 25 "114 25" _null_ _null_ "{from_json, field_name}" _null_ json_object_field_text _null_ _null_ _null_ )); @@ -4605,6 +4607,9 @@ DESCR("I/O"); DATA(insert OID = 3803 ( jsonb_send PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 17 "3802" _null_ _null_ _null_ _null_ jsonb_send _null_ _null_ _null_ )); DESCR("I/O"); +DATA(insert OID = 3301 ( jsonb_strip_nulls PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3802 "3802" _null_ _null_ _null_ _null_ jsonb_strip_nulls _null_ _null_ _null_ )); +DESCR("remove object fields with null values from jsonb"); + DATA(insert OID = 3478 ( jsonb_object_field PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 3802 "3802 25" _null_ _null_ "{from_json, field_name}" _null_ jsonb_object_field _null_ _null_ _null_ )); DATA(insert OID = 3214 ( jsonb_object_field_text PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 25 "3802 25" _null_ _null_ "{from_json, field_name}" _null_ jsonb_object_field_text _null_ _null_ _null_ )); DATA(insert OID = 3215 ( jsonb_array_element PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 3802 "3802 23" _null_ _null_ "{from_json, element_index}" _null_ jsonb_array_element _null_ _null_ _null_ )); diff --git a/src/include/utils/json.h b/src/include/utils/json.h index 82cc48b..cada47e 100644 --- a/src/include/utils/json.h +++ b/src/include/utils/json.h @@ -63,6 +63,7 @@ extern Datum json_populate_record(PG_FUNCTION_ARGS); extern Datum json_populate_recordset(PG_FUNCTION_ARGS); extern Datum json_to_record(PG_FUNCTION_ARGS); extern Datum json_to_recordset(PG_FUNCTION_ARGS); +extern Datum json_strip_nulls(PG_FUNCTION_ARGS); extern Datum jsonb_object_field(PG_FUNCTION_ARGS); extern Datum jsonb_object_field_text(PG_FUNCTION_ARGS); @@ -80,5 +81,6 @@ extern Datum jsonb_populate_record(PG_FUNCTION_ARGS); extern Datum jsonb_populate_recordset(PG_FUNCTION_ARGS); extern Datum jsonb_to_record(PG_FUNCTION_ARGS); extern Datum jsonb_to_recordset(PG_FUNCTION_ARGS); +extern Datum jsonb_strip_nulls(PG_FUNCTION_ARGS); #endif /* JSON_H */ diff --git a/src/test/regress/expected/json.out b/src/test/regress/expected/json.out index bb4d9ed..e435d3e 100644 --- a/src/test/regress/expected/json.out +++ b/src/test/regress/expected/json.out @@ -1586,3 +1586,53 @@ select * from json_to_recordset('[{"a":1,"b":{"d":"foo"},"c":true},{"a":2,"c":fa 2 | {"d":"bar"} | f (2 rows) +-- json_strip_nulls +select json_strip_nulls(null); + json_strip_nulls +------------------ + +(1 row) + +select json_strip_nulls('1'); + json_strip_nulls +------------------ + 1 +(1 row) + +select json_strip_nulls('"a string"'); + json_strip_nulls +------------------ + "a string" +(1 row) + +select json_strip_nulls('null'); + json_strip_nulls +------------------ + null +(1 row) + +select json_strip_nulls('[1,2,null,3,4]'); + json_strip_nulls +------------------ + [1,2,null,3,4] +(1 row) + +select json_strip_nulls('{"a":1,"b":null,"c":[2,null,3],"d":{"e":4,"f":null}}'); + json_strip_nulls +------------------------------------ + {"a":1,"c":[2,null,3],"d":{"e":4}} +(1 row) + +select json_strip_nulls('[1,{"a":1,"b":null,"c":2},3]'); + json_strip_nulls +--------------------- + [1,{"a":1,"c":2},3] +(1 row) + +-- an empty object is not null and should not be stripped +select json_strip_nulls('{"a": {"b": null, "c": null}, "d": {} }'); + json_strip_nulls +------------------ + {"a":{},"d":{}} +(1 row) + diff --git a/src/test/regress/expected/json_1.out b/src/test/regress/expected/json_1.out index 83c1d7d..106b481 100644 --- a/src/test/regress/expected/json_1.out +++ b/src/test/regress/expected/json_1.out @@ -1582,3 +1582,53 @@ select * from json_to_recordset('[{"a":1,"b":{"d":"foo"},"c":true},{"a":2,"c":fa 2 | {"d":"bar"} | f (2 rows) +-- json_strip_nulls +select json_strip_nulls(null); + json_strip_nulls +------------------ + +(1 row) + +select json_strip_nulls('1'); + json_strip_nulls +------------------ + 1 +(1 row) + +select json_strip_nulls('"a string"'); + json_strip_nulls +------------------ + "a string" +(1 row) + +select json_strip_nulls('null'); + json_strip_nulls +------------------ + null +(1 row) + +select json_strip_nulls('[1,2,null,3,4]'); + json_strip_nulls +------------------ + [1,2,null,3,4] +(1 row) + +select json_strip_nulls('{"a":1,"b":null,"c":[2,null,3],"d":{"e":4,"f":null}}'); + json_strip_nulls +------------------------------------ + {"a":1,"c":[2,null,3],"d":{"e":4}} +(1 row) + +select json_strip_nulls('[1,{"a":1,"b":null,"c":2},3]'); + json_strip_nulls +--------------------- + [1,{"a":1,"c":2},3] +(1 row) + +-- an empty object is not null and should not be stripped +select json_strip_nulls('{"a": {"b": null, "c": null}, "d": {} }'); + json_strip_nulls +------------------ + {"a":{},"d":{}} +(1 row) + diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out index 9146f59..3e1d769 100644 --- a/src/test/regress/expected/jsonb.out +++ b/src/test/regress/expected/jsonb.out @@ -2472,3 +2472,53 @@ SELECT '{"n":null,"a":1,"b":[1,2],"c":{"1":2},"d":{"1":[2,3]}}'::jsonb ? 'e'; f (1 row) +-- jsonb_strip_nulls +select jsonb_strip_nulls(null); + jsonb_strip_nulls +------------------- + +(1 row) + +select jsonb_strip_nulls('1'); + jsonb_strip_nulls +------------------- + 1 +(1 row) + +select jsonb_strip_nulls('"a string"'); + jsonb_strip_nulls +------------------- + "a string" +(1 row) + +select jsonb_strip_nulls('null'); + jsonb_strip_nulls +------------------- + null +(1 row) + +select jsonb_strip_nulls('[1,2,null,3,4]'); + jsonb_strip_nulls +-------------------- + [1, 2, null, 3, 4] +(1 row) + +select jsonb_strip_nulls('{"a":1,"b":null,"c":[2,null,3],"d":{"e":4,"f":null}}'); + jsonb_strip_nulls +-------------------------------------------- + {"a": 1, "c": [2, null, 3], "d": {"e": 4}} +(1 row) + +select jsonb_strip_nulls('[1,{"a":1,"b":null,"c":2},3]'); + jsonb_strip_nulls +-------------------------- + [1, {"a": 1, "c": 2}, 3] +(1 row) + +-- an empty object is not null and should not be stripped +select jsonb_strip_nulls('{"a": {"b": null, "c": null}, "d": {} }'); + jsonb_strip_nulls +-------------------- + {"a": {}, "d": {}} +(1 row) + diff --git a/src/test/regress/expected/jsonb_1.out b/src/test/regress/expected/jsonb_1.out index 83d61f8..35da6f4 100644 --- a/src/test/regress/expected/jsonb_1.out +++ b/src/test/regress/expected/jsonb_1.out @@ -2472,3 +2472,53 @@ SELECT '{"n":null,"a":1,"b":[1,2],"c":{"1":2},"d":{"1":[2,3]}}'::jsonb ? 'e'; f (1 row) +-- jsonb_strip_nulls +select jsonb_strip_nulls(null); + jsonb_strip_nulls +------------------- + +(1 row) + +select jsonb_strip_nulls('1'); + jsonb_strip_nulls +------------------- + 1 +(1 row) + +select jsonb_strip_nulls('"a string"'); + jsonb_strip_nulls +------------------- + "a string" +(1 row) + +select jsonb_strip_nulls('null'); + jsonb_strip_nulls +------------------- + null +(1 row) + +select jsonb_strip_nulls('[1,2,null,3,4]'); + jsonb_strip_nulls +-------------------- + [1, 2, null, 3, 4] +(1 row) + +select jsonb_strip_nulls('{"a":1,"b":null,"c":[2,null,3],"d":{"e":4,"f":null}}'); + jsonb_strip_nulls +-------------------------------------------- + {"a": 1, "c": [2, null, 3], "d": {"e": 4}} +(1 row) + +select jsonb_strip_nulls('[1,{"a":1,"b":null,"c":2},3]'); + jsonb_strip_nulls +-------------------------- + [1, {"a": 1, "c": 2}, 3] +(1 row) + +-- an empty object is not null and should not be stripped +select jsonb_strip_nulls('{"a": {"b": null, "c": null}, "d": {} }'); + jsonb_strip_nulls +-------------------- + {"a": {}, "d": {}} +(1 row) + diff --git a/src/test/regress/sql/json.sql b/src/test/regress/sql/json.sql index c980132..36a6674 100644 --- a/src/test/regress/sql/json.sql +++ b/src/test/regress/sql/json.sql @@ -509,3 +509,23 @@ select * from json_to_recordset('[{"a":1,"b":"foo","d":false},{"a":2,"b":"bar"," select * from json_to_recordset('[{"a":1,"b":{"d":"foo"},"c":true},{"a":2,"c":false,"b":{"d":"bar"}}]') as x(a int, b json, c boolean); + + +-- json_strip_nulls + +select json_strip_nulls(null); + +select json_strip_nulls('1'); + +select json_strip_nulls('"a string"'); + +select json_strip_nulls('null'); + +select json_strip_nulls('[1,2,null,3,4]'); + +select json_strip_nulls('{"a":1,"b":null,"c":[2,null,3],"d":{"e":4,"f":null}}'); + +select json_strip_nulls('[1,{"a":1,"b":null,"c":2},3]'); + +-- an empty object is not null and should not be stripped +select json_strip_nulls('{"a": {"b": null, "c": null}, "d": {} }'); diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql index f1ed021..e8e6117 100644 --- a/src/test/regress/sql/jsonb.sql +++ b/src/test/regress/sql/jsonb.sql @@ -542,3 +542,22 @@ SELECT '{"n":null,"a":1,"b":[1,2],"c":{"1":2},"d":{"1":[2,3]}}'::jsonb ? 'b'; SELECT '{"n":null,"a":1,"b":[1,2],"c":{"1":2},"d":{"1":[2,3]}}'::jsonb ? 'c'; SELECT '{"n":null,"a":1,"b":[1,2],"c":{"1":2},"d":{"1":[2,3]}}'::jsonb ? 'd'; SELECT '{"n":null,"a":1,"b":[1,2],"c":{"1":2},"d":{"1":[2,3]}}'::jsonb ? 'e'; + +-- jsonb_strip_nulls + +select jsonb_strip_nulls(null); + +select jsonb_strip_nulls('1'); + +select jsonb_strip_nulls('"a string"'); + +select jsonb_strip_nulls('null'); + +select jsonb_strip_nulls('[1,2,null,3,4]'); + +select jsonb_strip_nulls('{"a":1,"b":null,"c":[2,null,3],"d":{"e":4,"f":null}}'); + +select jsonb_strip_nulls('[1,{"a":1,"b":null,"c":2},3]'); + +-- an empty object is not null and should not be stripped +select jsonb_strip_nulls('{"a": {"b": null, "c": null}, "d": {} }');
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers