From 44cc027c44287439092f4669a57b4c1ea21753f5 Mon Sep 17 00:00:00 2001 From: jian he 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