v1-0001-make-JSON_QUERY-JSON_VALUE-returning-type-with.no-cfbot

application/octet-stream

Filename: v1-0001-make-JSON_QUERY-JSON_VALUE-returning-type-with.no-cfbot
Type: application/octet-stream
Part: 3
Message: Re: remaining sql/json patches
From 44cc027c44287439092f4669a57b4c1ea21753f5 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Mon, 22 Jan 2024 13:39:12 +0800
Subject: [PATCH v1 1/1] make JSON_QUERY|JSON_VALUE returning type with
 non-default typmod more consistent

---
 src/backend/executor/execExprInterp.c       | 14 +++++-
 src/backend/parser/parse_expr.c             | 54 +++++++++++++++++++--
 src/backend/utils/adt/jsonfuncs.c           |  1 +
 src/include/nodes/primnodes.h               |  1 +
 src/test/regress/expected/jsonb_sqljson.out | 34 +++++++++----
 src/test/regress/sql/jsonb_sqljson.sql      |  2 +-
 6 files changed, 89 insertions(+), 17 deletions(-)

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 964433a0..af25b903 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -4520,17 +4520,27 @@ ExecEvalJsonCoercion(ExprState *state, ExprEvalStep *op,
 	bool		resnull = *op->resnull;
 	Jsonb	   *jb = !resnull ? DatumGetJsonbP(res) : NULL;
 
+	if (!jb)
+	{
+		*op->resvalue = (Datum) 0;
+		return;
+	}
 	/*
 	 * Can't go to json_populate_type() for scalars when OMIT QUOTES is
 	 * specified, because it keeps the quotes by default.  So let's do the
 	 * deed ourselves by calling the input function, that is, after removing
 	 * the quotes.
 	 */
-	if (jb && JB_ROOT_IS_SCALAR(jb) && coercion->omit_quotes)
+	if (JB_ROOT_IS_SCALAR(jb) || coercion->via_io)
 	{
 		FmgrInfo   *input_finfo = op->d.jsonexpr_coercion.input_finfo;
 		Oid			typioparam = op->d.jsonexpr_coercion.typioparam;
-		char	   *val_string = JsonbUnquote(jb);
+		char	   *val_string;
+
+		if (coercion->omit_quotes)
+			val_string = JsonbUnquote(jb);
+		else
+			val_string = JsonbToCString(NULL, &jb->root, VARSIZE(jb));
 
 		(void) InputFunctionCallSafe(input_finfo, val_string, typioparam,
 									 coercion->targettypmod,
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 9b43b9e4..f59c177e 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -39,6 +39,7 @@
 #include "utils/fmgroids.h"
 #include "utils/jsonb.h"
 #include "utils/lsyscache.h"
+#include "utils/syscache.h"
 #include "utils/timestamp.h"
 #include "utils/xml.h"
 
@@ -4459,13 +4460,38 @@ coerceJsonFuncExprOutput(ParseState *pstate, JsonExpr *jsexpr)
 	 * Use a JsonCoercion node to implement a non-default QUOTES or WRAPPER
 	 * behavior.
 	 */
-	if (jsexpr->omit_quotes || jsexpr->wrapper != JSW_UNSPEC)
+	if (jsexpr->op == JSON_QUERY_OP)
 	{
-		JsonCoercion *coercion = makeJsonCoercion(returning);
+		Oid			returning_typid = returning->typid;
+		int32		returning_typmod = returning->typmod;
+		/*
+		* Use a JsonCoercion node to implement a non-default QUOTES or WRAPPER
+		* behavior.
+		* if returning type is domain then we use JsonCoercion node to do the coerce.
+		* if the domain based type typmod is not -1 (default) then we need CaseTestExpr.
+		* if returning type tyomod is not default (-1) then we use JsonCoercion.
+		*/
+		returning_typid = getBaseTypeAndTypmod(returning_typid, &returning_typmod);
 
-		coercion->omit_quotes = jsexpr->omit_quotes;
+		/* error out iff omit_quotes and non-default typmod */
+		if ((returning->typmod != -1 || returning_typmod != -1) && jsexpr->omit_quotes)
+			elog(ERROR, "JSON_QUERY returning type cannot have type modifier while specify OMIT QUOTES");
 
-		return (Node *) coercion;
+		/*
+		* by default domain itself typmod is -1.
+		* this iff branch deal with cases:
+		* returning type is not domain and the typmod is default.
+		* returning type is domain and the domain typmod is default.
+		*/
+		if ((returning->typmod == -1 && returning_typmod == -1) ||
+			(returning_typid != returning->typid && returning_typmod == -1))
+		{
+			JsonCoercion *coercion = makeJsonCoercion(returning);
+
+			coercion->omit_quotes = jsexpr->omit_quotes;
+
+			return (Node *) coercion;
+		}
 	}
 
 	default_typid = JsonFuncExprDefaultReturnType(jsexpr);
@@ -4519,7 +4545,27 @@ static JsonCoercion *
 makeJsonCoercion(const JsonReturning *returning)
 {
 	JsonCoercion *coercion = makeNode(JsonCoercion);
+	char		typtype;
+	HeapTuple	tp;
+	Form_pg_type typtup;
 
+	/*
+	* json_populate_type cannot handle domain check with soft error,
+	* so for domain type, we can only coerce via I/O.
+	* they are many cases, we need coerce via_io. but we need to make sure
+	* when type is RECORDOID and domain case it will be via_io.
+	* no need to worry about composite, since composite alway be quoted.
+	* also see ExecEvalJsonCoercion.
+	*/
+	tp = SearchSysCache1(TYPEOID, ObjectIdGetDatum(returning->typid));
+	if (!HeapTupleIsValid(tp))
+		elog(ERROR, "cache lookup failed for type %u", returning->typid);
+	typtup = (Form_pg_type) GETSTRUCT(tp);
+	typtype = typtup->typtype;
+	ReleaseSysCache(tp);
+
+	if (typtype == TYPTYPE_DOMAIN )
+		coercion->via_io = true;
 	coercion->targettype = returning->typid;
 	coercion->targettypmod = returning->typmod;
 
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 87599530..2e989d7b 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3392,6 +3392,7 @@ json_populate_type(Datum json_val, Oid json_type,
 			jsv.val.json.str = NULL;
 		else
 			jsv.val.jsonb = NULL;
+		return (Datum) 0;
 	}
 	else if (jsv.is_json)
 	{
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index fe9dfbb0..47a2bf5e 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1729,6 +1729,7 @@ typedef struct JsonCoercion
 	Oid			targettype;
 	int32		targettypmod;
 	bool		omit_quotes;	/* omit quotes from scalar output strings? */
+	bool		via_io;			/* via target type I/O function do the coercion */
 	Oid			collation;		/* collation for coercion via I/O or populate */
 } JsonCoercion;
 
diff --git a/src/test/regress/expected/jsonb_sqljson.out b/src/test/regress/expected/jsonb_sqljson.out
index a73105a4..b0c44cfc 100644
--- a/src/test/regress/expected/jsonb_sqljson.out
+++ b/src/test/regress/expected/jsonb_sqljson.out
@@ -301,7 +301,11 @@ SELECT JSON_VALUE(jsonb '"2017-02-20"', '$' RETURNING date) + 9;
 -- Test NULL checks execution in domain types
 CREATE DOMAIN sqljsonb_int_not_null AS int NOT NULL;
 SELECT JSON_VALUE(jsonb 'null', '$' RETURNING sqljsonb_int_not_null);
-ERROR:  domain sqljsonb_int_not_null does not allow null values
+ json_value 
+------------
+           
+(1 row)
+
 SELECT JSON_VALUE(jsonb 'null', '$' RETURNING sqljsonb_int_not_null ERROR ON ERROR);
 ERROR:  domain sqljsonb_int_not_null does not allow null values
 SELECT JSON_VALUE(jsonb 'null', '$' RETURNING sqljsonb_int_not_null DEFAULT 2 ON EMPTY ERROR ON ERROR);
@@ -313,7 +317,11 @@ SELECT JSON_VALUE(jsonb '1',  '$.a' RETURNING sqljsonb_int_not_null DEFAULT 2 ON
 (1 row)
 
 SELECT JSON_VALUE(jsonb '1',  '$.a' RETURNING sqljsonb_int_not_null DEFAULT NULL ON EMPTY ERROR ON ERROR);
-ERROR:  domain sqljsonb_int_not_null does not allow null values
+ json_value 
+------------
+           
+(1 row)
+
 CREATE TYPE rainbow AS ENUM ('red', 'orange', 'yellow', 'green', 'blue', 'purple');
 CREATE DOMAIN rgb AS rainbow CHECK (VALUE IN ('red', 'green', 'blue'));
 SELECT JSON_VALUE('"purple"'::jsonb, 'lax $[*]' RETURNING rgb);
@@ -653,7 +661,8 @@ SELECT JSON_QUERY(jsonb'{"rec": "{1,2,3}"}', '$.rec' returning int[] keep quotes
 (1 row)
 
 SELECT JSON_QUERY(jsonb'{"rec": "{1,2,3}"}', '$.rec' returning int[] keep quotes error on error);
-ERROR:  expected JSON array
+ERROR:  malformed array literal: ""{1,2,3}""
+DETAIL:  Array value must start with "{" or dimension information.
 SELECT JSON_QUERY(jsonb'{"rec": "[1,2]"}', '$.rec' returning int4range omit quotes);
  json_query 
 ------------
@@ -873,7 +882,8 @@ SELECT JSON_QUERY(jsonb'{"rec": "(abc,42,01.02.2003)"}', '$.rec' returning comp_
 (1 row)
 
 SELECT JSON_QUERY(jsonb'{"rec": "(abc,42,01.02.2003)"}', '$.rec' returning comp_abc keep quotes error on error);
-ERROR:  cannot call populate_composite on a scalar
+ERROR:  malformed record literal: ""(abc,42,01.02.2003)""
+DETAIL:  Missing left parenthesis.
 DROP TYPE comp_abc;
 -- Extension: record types returning
 CREATE TYPE sqljsonb_rec AS (a int, t text, js json, jb jsonb, jsa json[]);
@@ -945,7 +955,11 @@ SELECT JSON_QUERY(jsonb '{"a": 1}', '$.a' RETURNING sqljsonb_int_not_null);
 (1 row)
 
 SELECT JSON_QUERY(jsonb '{"a": 1}', '$.b' RETURNING sqljsonb_int_not_null);
-ERROR:  domain sqljsonb_int_not_null does not allow null values
+ json_query 
+------------
+           
+(1 row)
+
 -- Test timestamptz passing and output
 SELECT JSON_QUERY(jsonb 'null', '$ts' PASSING timestamptz '2018-02-21 12:34:56 +10' AS ts);
          json_query          
@@ -979,7 +993,7 @@ CREATE TABLE test_jsonb_constraints (
 	CONSTRAINT test_jsonb_constraint4
 		CHECK (JSON_QUERY(js::jsonb, '$.a' WITH CONDITIONAL WRAPPER EMPTY OBJECT ON ERROR) < jsonb '[10]')
 	CONSTRAINT test_jsonb_constraint5
-		CHECK (JSON_QUERY(js::jsonb, '$.a' RETURNING char(5) OMIT QUOTES EMPTY ARRAY ON EMPTY) >  'a' COLLATE "C")
+		CHECK (JSON_QUERY(js::jsonb, '$.a' RETURNING char(5) KEEP QUOTES EMPTY ARRAY ON EMPTY) >  'a' COLLATE "C")
 );
 \d test_jsonb_constraints
                                           Table "public.test_jsonb_constraints"
@@ -993,15 +1007,15 @@ Check constraints:
     "test_jsonb_constraint2" CHECK (JSON_EXISTS(js::jsonb, '$."a"' PASSING i + 5 AS int, i::text AS txt, ARRAY[1, 2, 3] AS arr))
     "test_jsonb_constraint3" CHECK (JSON_VALUE(js::jsonb, '$."a"' RETURNING integer DEFAULT 12 ON EMPTY ERROR ON ERROR) > i)
     "test_jsonb_constraint4" CHECK (JSON_QUERY(js::jsonb, '$."a"' RETURNING jsonb WITH CONDITIONAL WRAPPER EMPTY OBJECT ON ERROR) < '[10]'::jsonb)
-    "test_jsonb_constraint5" CHECK (JSON_QUERY(js::jsonb, '$."a"' RETURNING character(5) OMIT QUOTES EMPTY ARRAY ON EMPTY) > ('a'::bpchar COLLATE "C"))
+    "test_jsonb_constraint5" CHECK (JSON_QUERY(js::jsonb, '$."a"' RETURNING character(5) EMPTY ARRAY ON EMPTY) > ('a'::bpchar COLLATE "C"))
 
 SELECT check_clause
 FROM information_schema.check_constraints
 WHERE constraint_name LIKE 'test_jsonb_constraint%'
 ORDER BY 1;
-                                                      check_clause                                                      
-------------------------------------------------------------------------------------------------------------------------
- (JSON_QUERY((js)::jsonb, '$."a"' RETURNING character(5) OMIT QUOTES EMPTY ARRAY ON EMPTY) > ('a'::bpchar COLLATE "C"))
+                                                   check_clause                                                    
+-------------------------------------------------------------------------------------------------------------------
+ (JSON_QUERY((js)::jsonb, '$."a"' RETURNING character(5) EMPTY ARRAY ON EMPTY) > ('a'::bpchar COLLATE "C"))
  (JSON_QUERY((js)::jsonb, '$."a"' RETURNING jsonb WITH CONDITIONAL WRAPPER EMPTY OBJECT ON ERROR) < '[10]'::jsonb)
  (JSON_VALUE((js)::jsonb, '$."a"' RETURNING integer DEFAULT 12 ON EMPTY ERROR ON ERROR) > i)
  (js IS JSON)
diff --git a/src/test/regress/sql/jsonb_sqljson.sql b/src/test/regress/sql/jsonb_sqljson.sql
index b64c9017..48c3c8a1 100644
--- a/src/test/regress/sql/jsonb_sqljson.sql
+++ b/src/test/regress/sql/jsonb_sqljson.sql
@@ -302,7 +302,7 @@ CREATE TABLE test_jsonb_constraints (
 	CONSTRAINT test_jsonb_constraint4
 		CHECK (JSON_QUERY(js::jsonb, '$.a' WITH CONDITIONAL WRAPPER EMPTY OBJECT ON ERROR) < jsonb '[10]')
 	CONSTRAINT test_jsonb_constraint5
-		CHECK (JSON_QUERY(js::jsonb, '$.a' RETURNING char(5) OMIT QUOTES EMPTY ARRAY ON EMPTY) >  'a' COLLATE "C")
+		CHECK (JSON_QUERY(js::jsonb, '$.a' RETURNING char(5) KEEP QUOTES EMPTY ARRAY ON EMPTY) >  'a' COLLATE "C")
 );
 
 \d test_jsonb_constraints
-- 
2.34.1