Re: jsonb_set() strictness considered harmful to data

Pavel Stehule <[email protected]>
Newsgroups gmane.comp.db.postgresql.general,gmane.comp.db.postgresql.devel.general
Message-ID <CAFj8pRDPGAedFbmM+NOaJqgWNPBb0BikOmDztqLKxU8Y1amjMQ@mail.gmail.com> 
Hi

po 6. 1. 2020 v 22:34 odesílatel Andrew Dunstan <
[email protected]> napsal:

> On Thu, Nov 28, 2019 at 2:15 PM Andrew Dunstan
> <[email protected]> wrote:
> >
> >
> > On 11/27/19 9:35 PM, Michael Paquier wrote:
> > > On Fri, Nov 15, 2019 at 09:45:59PM +0100, Pavel Stehule wrote:
> > >> Maybe ERRCODE_NULL_VALUE_NOT_ALLOWED, and "NULL is not allowed",
> > >> errdetail - a exception due setting "null_value_treatment" =>
> > >> raise_exception
> > >> and maybe some errhint - "Maybe you would to use Jsonb NULL -
> "null"::jsonb"
> > >>
> > >> I don't know, but in this case, the exception should be verbose. This
> is
> > >> "rich" function with lot of functionality
> > > @Andrew: This patch is waiting on input from you for a couple of days
> > > now.
> > >
> >
> >
>
>
> Updated version including docco and better error message.
>
> cheers
>
> andrew
>

I think so my objections are solved. I have small objection

+ errdetail("exception raised due to \"null_value_treatment :=
'raise_exception'\""),
+ errhint("to avoid, either change the null_value_treatment argument or
ensure that an SQL NULL is not used")));

"null_value_treatment := 'raise_exception'\""

it use proprietary PostgreSQL syntax for named parameters. Better to use
ANSI/SQL syntax

"null_value_treatment => 'raise_exception'\""

It is fixed in attached patch

source compilation without warnings,
compilation docs without warnings
check-world passed without any problems

I'll mark this patch as ready for commiter

Thank you for your work

Pavel


>
> --
> Andrew Dunstan                https://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
jsonb_set_lax-3.patch (text/x-patch, 9.3 KB) 
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4b42f12862..72072e7545 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -12231,6 +12231,9 @@ table2-mapping
   <indexterm>
    <primary>jsonb_set</primary>
   </indexterm>
+  <indexterm>
+   <primary>jsonb_set_lax</primary>
+  </indexterm>
   <indexterm>
    <primary>jsonb_insert</primary>
   </indexterm>
@@ -12545,6 +12548,26 @@ table2-mapping
          </para><para><literal>[{"f1": 1, "f2": null, "f3": [2, 3, 4]}, 2]</literal>
         </para></entry>
        </row>
+      <row>
+       <entry><para><literal>jsonb_set_lax(target jsonb, path text[], new_value jsonb <optional>, create_missing boolean</optional> <optional>, null_value_treatment text</optional>)</literal>
+         </para></entry>
+       <entry><para><type>jsonb</type></para></entry>
+       <entry>
+        If <replaceable>new_value</replaceable> is not <literal>null</literal>,
+        behaves identically to <literal>jsonb_set</literal>. Otherwise behaves
+        according to the value of <replaceable>null_value_treatment</replaceable>
+        which must be one of <literal>'raise_exception'</literal>,
+        <literal>'use_json_null'</literal>, <literal>'delete_key'</literal>, or
+        <literal>'return_target'</literal>. The default is
+        <literal>'use_json_null'</literal>.
+       </entry>
+       <entry><para><literal>jsonb_set_lax('[{"f1":1,"f2":null},2,null,3]', '{0,f1}',null)</literal>
+         </para><para><literal>jsonb_set_lax('[{"f1":99,"f2":null},2]', '{0,f3}',null, true, 'return_target')</literal>
+         </para></entry>
+       <entry><para><literal>[{"f1":null,"f2":null},2,null,3]</literal>
+         </para><para><literal>[{"f1": 99, "f2": null}, 2]</literal>
+        </para></entry>
+       </row>
       <row>
        <entry>
            <para><literal>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 2fc3e3ff90..1cb2af1bcd 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1237,6 +1237,15 @@ LANGUAGE INTERNAL
 STRICT IMMUTABLE PARALLEL SAFE
 AS 'jsonb_set';
 
+CREATE OR REPLACE FUNCTION
+  jsonb_set_lax(jsonb_in jsonb, path text[] , replacement jsonb,
+            create_if_missing boolean DEFAULT true,
+            null_value_treatment text DEFAULT 'use_json_null')
+RETURNS jsonb
+LANGUAGE INTERNAL
+CALLED ON NULL INPUT IMMUTABLE PARALLEL SAFE
+AS 'jsonb_set_lax';
+
 CREATE OR REPLACE FUNCTION
   parse_ident(str text, strict boolean DEFAULT true)
 RETURNS text[]
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index ab5a24a858..4b5a0214dc 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -4395,6 +4395,70 @@ jsonb_set(PG_FUNCTION_ARGS)
 }
 
 
+/*
+ * SQL function jsonb_set_lax(jsonb, text[], jsonb, boolean, text)
+ */
+Datum
+jsonb_set_lax(PG_FUNCTION_ARGS)
+{
+	/* Jsonb	   *in = PG_GETARG_JSONB_P(0); */
+	/* ArrayType  *path = PG_GETARG_ARRAYTYPE_P(1); */
+	/* Jsonb	  *newval = PG_GETARG_JSONB_P(2); */
+	/* bool		create = PG_GETARG_BOOL(3); */
+	text       *handle_null;
+	char       *handle_val;
+
+	if (PG_ARGISNULL(0) || PG_ARGISNULL(1) || PG_ARGISNULL(3))
+		PG_RETURN_NULL();
+
+	/* could happen if they pass in an explicit NULL */
+	if (PG_ARGISNULL(4))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("need delete_key, return_target, use_json_null, or raise_exception")));
+
+	/* if the new value isn't an SQL NULL just call jsonb_set */
+	if (! PG_ARGISNULL(2))
+		return jsonb_set(fcinfo);
+
+	handle_null = PG_GETARG_TEXT_P(4);
+	handle_val = text_to_cstring(handle_null);
+
+	if (strcmp(handle_val,"raise_exception") == 0)
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+				 errmsg("NULL is not allowed"),
+				 errdetail("exception raised due to \"null_value_treatment => 'raise_exception'\""),
+				 errhint("to avoid, either change the null_value_treatment argument or ensure that an SQL NULL is not used")));
+	}
+	else if (strcmp(handle_val, "use_json_null") == 0)
+	{
+		Datum	  newval;
+
+		newval = DirectFunctionCall1(jsonb_in, CStringGetDatum("null"));
+
+		fcinfo->args[2].value = newval;
+		fcinfo->args[2].isnull = false;
+		return jsonb_set(fcinfo);
+	}
+	else if (strcmp(handle_val, "delete_key") == 0)
+	{
+		return jsonb_delete_path(fcinfo);
+	}
+	else if (strcmp(handle_val, "return_target") == 0)
+	{
+		Jsonb	   *in = PG_GETARG_JSONB_P(0);
+		PG_RETURN_JSONB_P(in);
+	}
+	else
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("need delete_key, return_target, use_json_null, or raise_exception")));
+	}
+}
+
 /*
  * SQL function jsonb_delete_path(jsonb, text[])
  */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 59f1ff01ab..3d2bf4e847 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -9296,6 +9296,9 @@
 { oid => '3304',
   proname => 'jsonb_delete_path', prorettype => 'jsonb',
   proargtypes => 'jsonb _text', prosrc => 'jsonb_delete_path' },
+{ oid => '8945', descr => 'Set part of a jsonb, handle NULL value',
+  proname => 'jsonb_set_lax', prorettype => 'jsonb', proisstrict => 'f',
+  proargtypes => 'jsonb _text jsonb bool text', prosrc => 'jsonb_set_lax' },
 { oid => '3305', descr => 'Set part of a jsonb',
   proname => 'jsonb_set', prorettype => 'jsonb',
   proargtypes => 'jsonb _text jsonb bool', prosrc => 'jsonb_set' },
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index a2a19f8104..b92f8e8dbc 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -4511,6 +4511,63 @@ select jsonb_set('{"a": {"b": [1, 2, 3]}}', '{a, b, non_integer}', '"new_value"'
 ERROR:  path element at position 3 is not an integer: "non_integer"
 select jsonb_set('{"a": {"b": [1, 2, 3]}}', '{a, b, NULL}', '"new_value"');
 ERROR:  path element at position 3 is null
+-- jsonb_set_lax
+\pset null NULL
+-- pass though non nulls to jsonb_set
+select jsonb_set_lax('{"a":1,"b":2}','{b}','5') ;
+  jsonb_set_lax   
+------------------
+ {"a": 1, "b": 5}
+(1 row)
+
+select jsonb_set_lax('{"a":1,"b":2}','{d}','6', true) ;
+      jsonb_set_lax       
+--------------------------
+ {"a": 1, "b": 2, "d": 6}
+(1 row)
+
+-- using the default treatment
+select jsonb_set_lax('{"a":1,"b":2}','{b}',null);
+    jsonb_set_lax    
+---------------------
+ {"a": 1, "b": null}
+(1 row)
+
+select jsonb_set_lax('{"a":1,"b":2}','{d}',null,true);
+        jsonb_set_lax        
+-----------------------------
+ {"a": 1, "b": 2, "d": null}
+(1 row)
+
+-- errors
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, true, null);
+ERROR:  need delete_key, return_target, use_json_null, or raise_exception
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, true, 'no_such_treatment');
+ERROR:  need delete_key, return_target, use_json_null, or raise_exception
+-- explicit treatments
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'raise_exception') as raise_exception;
+ERROR:  NULL is not allowed
+DETAIL:  exception raised due to "null_value_treatment => 'raise_exception'"
+HINT:  to avoid, either change the null_value_treatment argument or ensure that an SQL NULL is not used
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'return_target') as return_target;
+  return_target   
+------------------
+ {"a": 1, "b": 2}
+(1 row)
+
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'delete_key') as delete_key;
+ delete_key 
+------------
+ {"a": 1}
+(1 row)
+
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'use_json_null') as use_json_null;
+    use_json_null    
+---------------------
+ {"a": 1, "b": null}
+(1 row)
+
+\pset null
 -- jsonb_insert
 select jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"');
          jsonb_insert          
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index efd4c45185..3e2b8f66df 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -1153,6 +1153,26 @@ select jsonb_set('{"a": [1, 2, 3]}', '{a, non_integer}', '"new_value"');
 select jsonb_set('{"a": {"b": [1, 2, 3]}}', '{a, b, non_integer}', '"new_value"');
 select jsonb_set('{"a": {"b": [1, 2, 3]}}', '{a, b, NULL}', '"new_value"');
 
+-- jsonb_set_lax
+
+\pset null NULL
+
+-- pass though non nulls to jsonb_set
+select jsonb_set_lax('{"a":1,"b":2}','{b}','5') ;
+select jsonb_set_lax('{"a":1,"b":2}','{d}','6', true) ;
+-- using the default treatment
+select jsonb_set_lax('{"a":1,"b":2}','{b}',null);
+select jsonb_set_lax('{"a":1,"b":2}','{d}',null,true);
+-- errors
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, true, null);
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, true, 'no_such_treatment');
+-- explicit treatments
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'raise_exception') as raise_exception;
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'return_target') as return_target;
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'delete_key') as delete_key;
+select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'use_json_null') as use_json_null;
+
+\pset null
 
 -- jsonb_insert
 select jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"');
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.