From 529f85684774cb70e85af5671a7edf3526b96393 Mon Sep 17 00:00:00 2001 From: Nikita Malakhov Date: Mon, 16 Oct 2023 00:24:01 +0300 Subject: [PATCH] Add check whether it is needed to override default coercion in JSON_QUERY function with OMIT QUOTES case, and checks is scalar types have error safe input/coercion functions, with test cases correction according to slightly changed behavior --- src/backend/executor/execExprInterp.c | 33 +++++++ src/backend/parser/parse_expr.c | 96 ++++++++++++++++++--- src/test/regress/expected/jsonb_sqljson.out | 43 ++++++--- src/test/regress/sql/jsonb_sqljson.sql | 16 +++- 4 files changed, 162 insertions(+), 26 deletions(-) diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 9adf31682c..cd92f920e4 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -4176,6 +4176,28 @@ ExecEvalJsonIsPredicate(ExprState *state, ExprEvalStep *op) *op->resvalue = BoolGetDatum(res); } +/* + * Check whether we need to override default coercion in + * JSON_QUERY(OMIT QUOTES) case. + */ +static bool +ExecJsonQueryNeedsIOCoercion(JsonExpr *jsexpr, Datum *res, bool isnull) +{ + if (jsexpr->omit_quotes && !isnull) + { + Jsonb *jb = DatumGetJsonbP(*res); + JsonbValue jbv; + + /* Coerce non-null scalar items via I/O in OMIT QUOTES case */ + if (JB_ROOT_IS_SCALAR(jb) && + JsonbExtractScalar(&jb->root, &jbv) && + jbv.type == jbvString) + return true; + } + + return false; +} + /* * Evaluate given JsonExpr by performing the specified JSON operation. * @@ -4236,7 +4258,18 @@ ExecEvalJsonExpr(ExprState *state, ExprEvalStep *op, ExprContext *econtext) *op->resvalue = (Datum) 0; return; } + resnull = !DatumGetPointer(res); + + /* Check coercion in OMIT QUOTES case */ + if (post_eval->jcstate && post_eval->jcstate->coercion && + post_eval->jcstate->coercion->via_io && + jexpr->on_error && jexpr->on_error->btype != JSON_BEHAVIOR_NULL) + if(!ExecJsonQueryNeedsIOCoercion(jexpr, &res, resnull)) + ereport(ERROR, + (errcode(ERRCODE_SQL_JSON_ITEM_CANNOT_BE_CAST_TO_TARGET_TYPE), + errmsg("SQL/JSON item cannot be cast to target type"))); + break; case JSON_VALUE_OP: diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index e62794dee5..58549461a4 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -4304,7 +4304,6 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) ret->typid = JsonFuncExprDefaultReturnType(jsexpr); ret->typmod = -1; } - jsexpr->result_coercion = coerceJsonFuncExprOutput(pstate, jsexpr); jsexpr->on_empty = transformJsonBehavior(pstate, func->on_empty, JSON_BEHAVIOR_NULL, @@ -4312,6 +4311,9 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) jsexpr->on_error = transformJsonBehavior(pstate, func->on_error, JSON_BEHAVIOR_NULL, jsexpr->returning); + + jsexpr->result_coercion = coerceJsonFuncExprOutput(pstate, jsexpr); + break; case JSON_VALUE_OP: @@ -4325,6 +4327,14 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) jsexpr->returning->typid = TEXTOID; jsexpr->returning->typmod = -1; } + + jsexpr->on_empty = transformJsonBehavior(pstate, func->on_empty, + JSON_BEHAVIOR_NULL, + jsexpr->returning); + jsexpr->on_error = transformJsonBehavior(pstate, func->on_error, + JSON_BEHAVIOR_NULL, + jsexpr->returning); + jsexpr->result_coercion = coerceJsonFuncExprOutput(pstate, jsexpr); /* @@ -4335,12 +4345,6 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) InitJsonItemCoercions(pstate, jsexpr->returning, exprType(jsexpr->formatted_expr)); - jsexpr->on_empty = transformJsonBehavior(pstate, func->on_empty, - JSON_BEHAVIOR_NULL, - jsexpr->returning); - jsexpr->on_error = transformJsonBehavior(pstate, func->on_error, - JSON_BEHAVIOR_NULL, - jsexpr->returning); break; } @@ -4416,6 +4420,38 @@ transformJsonExprCommon(ParseState *pstate, JsonFuncExpr *func, return jsexpr; } +/* + * Check whether scalar type is required by SQL/JSON standard and has + * error-safe input/conversion functions. + */ +static bool +scalarTypeIsErrorSafe(Oid typid) +{ + switch (typid) + { + case JSONBOID: + case JSONOID: + case TEXTOID: + case VARCHAROID: + case BPCHAROID: + case BOOLOID: + case NUMERICOID: + case INT2OID: + case INT4OID: + case INT8OID: + case FLOAT4OID: + case FLOAT8OID: + case DATEOID: + case TIMEOID: + case TIMETZOID: + case TIMESTAMPOID: + case TIMESTAMPTZOID: + return true; + default: + return false; + } +} + /* * Transform a JSON PASSING clause. */ @@ -4452,6 +4488,11 @@ coerceJsonFuncExprOutput(ParseState *pstate, JsonExpr *jsexpr) Node *context_item = jsexpr->formatted_expr; int default_typmod; Oid default_typid; + CaseTestExpr *placeholder = makeNode(CaseTestExpr); + + placeholder->typeId = exprType(context_item); + placeholder->typeMod = exprTypmod(context_item); + placeholder->collation = exprCollation(context_item); Assert(returning); @@ -4462,6 +4503,29 @@ coerceJsonFuncExprOutput(ParseState *pstate, JsonExpr *jsexpr) if (returning->typid != JSONOID && returning->typid != JSONBOID && (jsexpr->op != JSON_QUERY_OP || jsexpr->omit_quotes)) { + if (!scalarTypeIsErrorSafe(getBaseType(returning->typid)) && + (!jsexpr->on_error || + jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR)) + { + if (jsexpr->op == JSON_QUERY_OP) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("returning type %s is not supported in JSON_QUERY() without ERROR ON ERROR", + format_type_be(jsexpr->returning->typid)), + parser_coercion_errposition(pstate, + exprLocation((Node *)placeholder), + (Node *) jsexpr))); + + if (jsexpr->op == JSON_VALUE_OP) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("returning type %s is not supported in JSON_VALUE() without ERROR ON ERROR", + format_type_be(jsexpr->returning->typid)), + parser_coercion_errposition(pstate, + exprLocation((Node *)placeholder), + (Node *) jsexpr))); + } + coercion = makeNode(JsonCoercion); coercion->expr = NULL; coercion->via_io = true; @@ -4487,16 +4551,24 @@ coerceJsonFuncExprOutput(ParseState *pstate, JsonExpr *jsexpr) * evaluating the JSON_VALUE/QUERY jsonpath expression to the coercion * function. */ - CaseTestExpr *placeholder = makeNode(CaseTestExpr); - - placeholder->typeId = exprType(context_item); - placeholder->typeMod = exprTypmod(context_item); - placeholder->collation = exprCollation(context_item); Assert(placeholder->typeId == default_typid); Assert(placeholder->typeMod == default_typmod); coercion = coerceJsonExpr(pstate, (Node *) placeholder, returning); + + if (coercion->via_io && !jsexpr->omit_quotes) + { + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot cast type %s to %s", + format_type_be(exprType(context_item)), + format_type_be(jsexpr->returning->typid)), + errhint("Try to use OMIT QUOTES clause in JSON_QUERY()."), + parser_coercion_errposition(pstate, + exprLocation((Node *)placeholder), + (Node *) jsexpr))); + } } return coercion; diff --git a/src/test/regress/expected/jsonb_sqljson.out b/src/test/regress/expected/jsonb_sqljson.out index 8dcdb90fc8..e9ff4f67b1 100644 --- a/src/test/regress/expected/jsonb_sqljson.out +++ b/src/test/regress/expected/jsonb_sqljson.out @@ -358,20 +358,28 @@ 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 '1', '$.a' RETURNING sqljsonb_int_not_null); +SELECT JSON_VALUE(jsonb 'null', '$' RETURNING sqljsonb_int_not_null); ERROR: domain sqljsonb_int_not_null does not allow null values -SELECT JSON_VALUE(jsonb '1', '$.a' RETURNING sqljsonb_int_not_null NULL ON ERROR); +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 '1', '$.a' RETURNING sqljsonb_int_not_null DEFAULT NULL ON ERROR); +SELECT JSON_VALUE(jsonb 'null', '$' RETURNING sqljsonb_int_not_null DEFAULT 2 ON EMPTY ERROR ON ERROR); ERROR: domain sqljsonb_int_not_null does not allow null values -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); +SELECT JSON_VALUE(jsonb '1', '$.a' RETURNING sqljsonb_int_not_null DEFAULT 2 ON EMPTY ERROR ON ERROR); json_value ------------ - + 2 (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 +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); +ERROR: returning type rgb is not supported in JSON_VALUE() without ERROR ON ERROR +LINE 1: SELECT JSON_VALUE('"purple"'::jsonb, 'lax $[*]' RETURNING rg... + ^ +SELECT JSON_VALUE('"purple"'::jsonb, 'lax $[*]' RETURNING rgb ERROR ON ERROR); +ERROR: value for domain rgb violates check constraint "rgb_check" SELECT JSON_VALUE(jsonb '[]', '$'); json_value ------------ @@ -500,6 +508,10 @@ SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a); (1 row) SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a RETURNING point); +ERROR: returning type point is not supported in JSON_VALUE() without ERROR ON ERROR +LINE 1: SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )... + ^ +SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a RETURNING point ERROR ON ERROR); json_value ------------ (1,2) @@ -907,6 +919,17 @@ SELECT * FROM unnest((JSON_QUERY(jsonb '{"reca": [{"a": 1, "t": ["foo", []]}, {" 2 | | | [{}, true] | (2 rows) +SELECT JSON_QUERY(jsonb '[{"a": 1, "b": "foo", "t": "aaa", "js": [1, "2", {}], "jb": {"x": [1, "2", {}]}}, {"a": 2}]', '$[0]' RETURNING jsonpath); +ERROR: cannot cast type jsonb to jsonpath +LINE 1: SELECT JSON_QUERY(jsonb '[{"a": 1, "b": "foo", "t": "aaa", "... + ^ +HINT: Try to use OMIT QUOTES clause in JSON_QUERY(). +SELECT JSON_QUERY(jsonb '[{"a": 1, "b": "foo", "t": "aaa", "js": [1, "2", {}], "jb": {"x": [1, "2", {}]}}, {"a": 2}]', '$[0]' RETURNING jsonpath OMIT QUOTES); +ERROR: returning type jsonpath is not supported in JSON_QUERY() without ERROR ON ERROR +LINE 1: SELECT JSON_QUERY(jsonb '[{"a": 1, "b": "foo", "t": "aaa", "... + ^ +SELECT JSON_QUERY(jsonb '[{"a": 1, "b": "foo", "t": "aaa", "js": [1, "2", {}], "jb": {"x": [1, "2", {}]}}, {"a": 2}]', '$[0]' RETURNING jsonpath OMIT QUOTES ERROR ON ERROR); +ERROR: SQL/JSON item cannot be cast to target type -- Extension: array types returning SELECT JSON_QUERY(jsonb '[1,2,null,"3"]', '$[*]' RETURNING int[] WITH WRAPPER); json_query @@ -967,7 +990,7 @@ CREATE TABLE test_jsonb_constraints ( CONSTRAINT test_jsonb_constraint2 CHECK (JSON_EXISTS(js::jsonb, '$.a' PASSING i + 5 AS int, i::text AS txt, array[1,2,3] as arr)) CONSTRAINT test_jsonb_constraint3 - CHECK (JSON_VALUE(js::jsonb, '$.a' RETURNING int DEFAULT ('12' || i)::int ON EMPTY ERROR ON ERROR) > i) + CHECK (JSON_VALUE(js::jsonb, '$.a' RETURNING int DEFAULT '12' ON EMPTY ERROR ON ERROR) > i) CONSTRAINT test_jsonb_constraint4 CHECK (JSON_QUERY(js::jsonb, '$.a' WITH CONDITIONAL WRAPPER EMPTY OBJECT ON ERROR) < jsonb '[10]') CONSTRAINT test_jsonb_constraint5 @@ -985,7 +1008,7 @@ CREATE TABLE test_jsonb_constraints ( Check constraints: "test_jsonb_constraint1" CHECK (js IS JSON) "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'::text || i)::integer ON EMPTY ERROR ON ERROR) > i) + "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_constraint6" CHECK (JSON_EXISTS(js::jsonb, 'strict $."a"' RETURNING integer TRUE ON ERROR) < 2) @@ -999,7 +1022,7 @@ ORDER BY 1; (JSON_EXISTS((js)::jsonb, 'strict $."a"' RETURNING integer TRUE ON ERROR) < 2) (JSON_QUERY((js)::jsonb, '$."a"' RETURNING character(5) OMIT QUOTES 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'::text || i))::integer ON EMPTY ERROR ON ERROR) > i) + (JSON_VALUE((js)::jsonb, '$."a"' RETURNING integer DEFAULT 12 ON EMPTY ERROR ON ERROR) > i) (js IS JSON) JSON_EXISTS((js)::jsonb, '$."a"' PASSING (i + 5) AS int, (i)::text AS txt, ARRAY[1, 2, 3] AS arr) (6 rows) diff --git a/src/test/regress/sql/jsonb_sqljson.sql b/src/test/regress/sql/jsonb_sqljson.sql index 6c3f8c7e43..a410369535 100644 --- a/src/test/regress/sql/jsonb_sqljson.sql +++ b/src/test/regress/sql/jsonb_sqljson.sql @@ -87,12 +87,15 @@ 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 '1', '$.a' RETURNING sqljsonb_int_not_null); -SELECT JSON_VALUE(jsonb '1', '$.a' RETURNING sqljsonb_int_not_null NULL ON ERROR); -SELECT JSON_VALUE(jsonb '1', '$.a' RETURNING sqljsonb_int_not_null DEFAULT NULL ON ERROR); +SELECT JSON_VALUE(jsonb 'null', '$' RETURNING sqljsonb_int_not_null); +SELECT JSON_VALUE(jsonb 'null', '$' RETURNING sqljsonb_int_not_null ERROR ON ERROR); +SELECT JSON_VALUE(jsonb 'null', '$' RETURNING sqljsonb_int_not_null DEFAULT 2 ON EMPTY ERROR ON ERROR); +SELECT JSON_VALUE(jsonb '1', '$.a' RETURNING sqljsonb_int_not_null DEFAULT 2 ON EMPTY ERROR ON ERROR); +SELECT JSON_VALUE(jsonb '1', '$.a' RETURNING sqljsonb_int_not_null DEFAULT NULL ON EMPTY ERROR ON ERROR); 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); +SELECT JSON_VALUE('"purple"'::jsonb, 'lax $[*]' RETURNING rgb ERROR ON ERROR); SELECT JSON_VALUE(jsonb '[]', '$'); SELECT JSON_VALUE(jsonb '[]', '$' ERROR ON ERROR); @@ -135,6 +138,7 @@ FROM SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a); SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a RETURNING point); +SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a RETURNING point ERROR ON ERROR); -- Test timestamptz passing and output SELECT JSON_VALUE(jsonb 'null', '$ts' PASSING timestamptz '2018-02-21 12:34:56 +10' AS ts); @@ -259,6 +263,10 @@ SELECT JSON_QUERY(jsonb '[{"a": "a", "b": "foo", "t": "aaa", "js": [1, "2", {}], SELECT * FROM unnest((JSON_QUERY(jsonb '{"jsa": [{"a": 1, "b": ["foo"]}, {"a": 2, "c": {}}, 123]}', '$' RETURNING sqljsonb_rec)).jsa); SELECT * FROM unnest((JSON_QUERY(jsonb '{"reca": [{"a": 1, "t": ["foo", []]}, {"a": 2, "jb": [{}, true]}]}', '$' RETURNING sqljsonb_reca)).reca); +SELECT JSON_QUERY(jsonb '[{"a": 1, "b": "foo", "t": "aaa", "js": [1, "2", {}], "jb": {"x": [1, "2", {}]}}, {"a": 2}]', '$[0]' RETURNING jsonpath); +SELECT JSON_QUERY(jsonb '[{"a": 1, "b": "foo", "t": "aaa", "js": [1, "2", {}], "jb": {"x": [1, "2", {}]}}, {"a": 2}]', '$[0]' RETURNING jsonpath OMIT QUOTES); +SELECT JSON_QUERY(jsonb '[{"a": 1, "b": "foo", "t": "aaa", "js": [1, "2", {}], "jb": {"x": [1, "2", {}]}}, {"a": 2}]', '$[0]' RETURNING jsonpath OMIT QUOTES ERROR ON ERROR); + -- Extension: array types returning SELECT JSON_QUERY(jsonb '[1,2,null,"3"]', '$[*]' RETURNING int[] WITH WRAPPER); SELECT JSON_QUERY(jsonb '[1,2,null,"a"]', '$[*]' RETURNING int[] WITH WRAPPER ERROR ON ERROR); @@ -285,7 +293,7 @@ CREATE TABLE test_jsonb_constraints ( CONSTRAINT test_jsonb_constraint2 CHECK (JSON_EXISTS(js::jsonb, '$.a' PASSING i + 5 AS int, i::text AS txt, array[1,2,3] as arr)) CONSTRAINT test_jsonb_constraint3 - CHECK (JSON_VALUE(js::jsonb, '$.a' RETURNING int DEFAULT ('12' || i)::int ON EMPTY ERROR ON ERROR) > i) + CHECK (JSON_VALUE(js::jsonb, '$.a' RETURNING int DEFAULT '12' ON EMPTY ERROR ON ERROR) > i) CONSTRAINT test_jsonb_constraint4 CHECK (JSON_QUERY(js::jsonb, '$.a' WITH CONDITIONAL WRAPPER EMPTY OBJECT ON ERROR) < jsonb '[10]') CONSTRAINT test_jsonb_constraint5 -- 2.25.1