v1-0001-SQL-JSON-Use-implicit-casts-for-RETURNING-type-wi.patch

application/octet-stream

Filename: v1-0001-SQL-JSON-Use-implicit-casts-for-RETURNING-type-wi.patch
Type: application/octet-stream
Part: 0
Message: Re: pgsql: Add more SQL/JSON constructor functions

Patch

Format: format-patch
Series: patch v1-0001
Subject: SQL/JSON: Use implicit casts for RETURNING type with typmod
File+
src/backend/parser/parse_expr.c 26 3
src/test/regress/expected/sqljson_jsontable.out 9 9
src/test/regress/expected/sqljson.out 11 0
src/test/regress/expected/sqljson_queryfuncs.out 15 11
src/test/regress/sql/sqljson_queryfuncs.sql 5 3
src/test/regress/sql/sqljson.sql 7 0
From 4bbd598aca05bbeb7dd9a4f9a8087a05533e439d Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Wed, 5 Jun 2024 09:13:15 +0900
Subject: [PATCH v1] SQL/JSON: Use implicit casts for RETURNING type with
 typmod

This fixes things so that the various SQL/JSON functions that support
specifying the type of the returned value using the RETURNING clause
perform an implicit cast to the target type at least in the cases
where the target type also specifies a length limit.  That is to
ensure that the output values that exceed the length produce an error
instead of being silently truncated to the specified length, a
behavior that agrees with what the standard says regarding this.

Reported-by: Alvaro Herrera
Discussion: https://postgr.es/m/202405271326.5a5rprki64aw%40alvherre.pgsql
---
 src/backend/parser/parse_expr.c               | 29 +++++++++++++++++--
 src/test/regress/expected/sqljson.out         | 11 +++++++
 .../regress/expected/sqljson_jsontable.out    | 18 ++++++------
 .../regress/expected/sqljson_queryfuncs.out   | 26 ++++++++++-------
 src/test/regress/sql/sqljson.sql              |  7 +++++
 src/test/regress/sql/sqljson_queryfuncs.sql   |  8 +++--
 6 files changed, 73 insertions(+), 26 deletions(-)

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 4c95986c31..afeb8cb1d2 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -3614,7 +3614,9 @@ coerceJsonFuncExpr(ParseState *pstate, Node *expr,
 	/* try to coerce expression to the output type */
 	res = coerce_to_target_type(pstate, expr, exprtype,
 								returning->typid, returning->typmod,
+								returning->typmod > 0 ? COERCION_IMPLICIT :
 								COERCION_EXPLICIT,
+								returning->typmod > 0 ? COERCE_IMPLICIT_CAST :
 								COERCE_EXPLICIT_CAST,
 								location);
 
@@ -3677,6 +3679,14 @@ makeJsonConstructorExpr(ParseState *pstate, JsonConstructorType type,
 		placeholder = (Node *) cte;
 	}
 
+	/*
+	 * Convert the source expression to text, because coerceJsonFuncExpr() will
+	 * create an implicit cast to the RETURNING types with typmod and there are
+	 * no implicit casts from json(b) to such types.
+	 */
+	if (returning->typmod > 0)
+		placeholder = coerce_to_specific_type(pstate, placeholder, TEXTOID,
+											  "JSON_CONSTRUCTOR()");
 	coercion = coerceJsonFuncExpr(pstate, placeholder, returning, true);
 
 	if (coercion != placeholder)
@@ -4526,10 +4536,9 @@ coerceJsonExprOutput(ParseState *pstate, JsonExpr *jsexpr)
 	if (jsexpr->op == JSON_VALUE_OP)
 	{
 		/*
-		 * Use cast expressions for types with typmod and domain types.
+		 * Use cast expression for domain types; we need CoerceToDomain here.
 		 */
-		if (returning->typmod == -1 &&
-			get_typtype(returning->typid) != TYPTYPE_DOMAIN)
+		if (get_typtype(returning->typid) != TYPTYPE_DOMAIN)
 		{
 			jsexpr->use_io_coercion = true;
 			return;
@@ -4561,6 +4570,20 @@ coerceJsonExprOutput(ParseState *pstate, JsonExpr *jsexpr)
 			default:
 				break;
 		}
+
+		/*
+		 * coerceJsonFuncExpr() creates implicit casts for types with typmod,
+		 * which (if present) don't handle errors softly, so use runtime
+		 * coercion.
+		 */
+		if (returning->typmod > 0)
+		{
+			if (jsexpr->omit_quotes)
+				jsexpr->use_io_coercion = true;
+			else
+				jsexpr->use_json_coercion = true;
+			return;
+		}
 	}
 
 	/* Look up a cast expression. */
diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out
index cbf8542d8d..034c22cb44 100644
--- a/src/test/regress/expected/sqljson.out
+++ b/src/test/regress/expected/sqljson.out
@@ -1278,3 +1278,14 @@ CREATE OR REPLACE VIEW public.is_json_view AS
     '{}'::text IS JSON OBJECT WITH UNIQUE KEYS AS object
    FROM generate_series(1, 3) i(i)
 DROP VIEW is_json_view;
+-- Test implicit coercion to a fixed-length type specified in RETURNING
+SELECT JSON_SERIALIZE('{ "a" : 1 } ' RETURNING varchar(2));
+ERROR:  value too long for type character varying(2)
+SELECT JSON_OBJECT('a': JSON_OBJECT('b': 1 RETURNING varchar(2)));
+ERROR:  value too long for type character varying(2)
+SELECT JSON_ARRAY(JSON_ARRAY('{ "a" : 123 }' RETURNING varchar(2)));
+ERROR:  value too long for type character varying(2)
+SELECT JSON_ARRAYAGG(('111' || i)::bytea FORMAT JSON NULL ON NULL RETURNING varchar(2)) FROM generate_series(1,1) i;
+ERROR:  value too long for type character varying(2)
+SELECT JSON_OBJECTAGG(i: ('111' || i)::bytea FORMAT JSON WITH UNIQUE RETURNING varchar(2)) FROM generate_series(1, 1) i;
+ERROR:  value too long for type character varying(2)
diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out
index 9f649483ce..2d74fba506 100644
--- a/src/test/regress/expected/sqljson_jsontable.out
+++ b/src/test/regress/expected/sqljson_jsontable.out
@@ -103,14 +103,14 @@ FROM json_table_test vals
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  1 |   1 | 1       | 1       | t    |       1 | 1       | 1            | 1
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  2 |     | 1.23    | 1.23    |      |    1.23 | 1.23    | 1.23         | 1.23
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  3 |   2 | 2       | 2       |      |       2 | 2       | "2"          | "2"
- [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  4 |     | aaaaaaa | aaaa    |      |         | aaaaaaa | "aaaaaaa"    | "aaaaaaa"
+ [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  4 |     | aaaaaaa |         |      |         | aaaaaaa | "aaaaaaa"    | "aaaaaaa"
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  5 |     | foo     | foo     |      |         |         | "foo"        | "foo"
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  6 |     |         |         |      |         |         | null         | null
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  7 |     | f       | f       | f    |         | f       | false        | false
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  8 |     | t       | t       | t    |         | t       | true         | true
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  9 |     |         |         |      |         |         | {"aaa": 123} | {"aaa": 123}
- [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 10 |     | [1,2]   | [1,2    |      |         | [1,2]   | "[1,2]"      | "[1,2]"
- [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 11 |     | "str"   | "str    |      |         | "str"   | "\"str\""    | "\"str\""
+ [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 10 |     | [1,2]   |         |      |         | [1,2]   | "[1,2]"      | "[1,2]"
+ [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 11 |     | "str"   |         |      |         | "str"   | "\"str\""    | "\"str\""
 (14 rows)
 
 -- "formatted" columns
@@ -137,14 +137,14 @@ FROM json_table_test vals
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  1 | 1            | 1    | 1    | 1            | 1
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  2 | 1.23         | 1.23 | 1.23 | 1.23         | 1.23
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  3 | "2"          | "2"  | "2"  | "2"          | 2
- [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  4 | "aaaaaaa"    | "aaa | "aaa | "aaaaaaa"    | 
- [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  5 | "foo"        | "foo | "foo | "foo"        | 
+ [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  4 | "aaaaaaa"    |      |      | "aaaaaaa"    | 
+ [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  5 | "foo"        |      |      | "foo"        | 
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  6 | null         | null | null | null         | null
- [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  7 | false        | fals | fals | false        | false
+ [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  7 | false        |      |      | false        | false
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  8 | true         | true | true | true         | true
- [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  9 | {"aaa": 123} | {"aa | {"aa | {"aaa": 123} | {"aaa": 123}
- [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 10 | "[1,2]"      | "[1, | "[1, | "[1,2]"      | [1, 2]
- [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 11 | "\"str\""    | "\"s | "\"s | "\"str\""    | "str"
+ [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] |  9 | {"aaa": 123} |      |      | {"aaa": 123} | {"aaa": 123}
+ [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 10 | "[1,2]"      |      |      | "[1,2]"      | [1, 2]
+ [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 11 | "\"str\""    |      |      | "\"str\""    | "str"
 (14 rows)
 
 -- EXISTS columns
diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index 98117b346d..24672524ad 100644
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -234,10 +234,18 @@ SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING char(5));
  aaa  
 (1 row)
 
+SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING char(2) ERROR ON ERROR);
+ERROR:  value too long for type character(2)
 SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING char(2));
  json_value 
 ------------
- aa
+ 
+(1 row)
+
+SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING char(3) ERROR ON ERROR);
+ json_value 
+------------
+ aaa
 (1 row)
 
 SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING json);
@@ -632,16 +640,18 @@ SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING bytea FORMAT JSON OMIT QUOTES ERR
 (1 row)
 
 -- Behavior when a RETURNING type has typmod != -1
-SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING char(2));
+SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING char(3) ERROR ON ERROR);
+ERROR:  value too long for type character(3)
+SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING char(3));
  json_query 
 ------------
- "a
+ 
 (1 row)
 
-SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING char(2) OMIT QUOTES);
+SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING char(3) OMIT QUOTES ERROR ON ERROR);
  json_query 
 ------------
- aa
+ aaa
 (1 row)
 
 SELECT JSON_QUERY(jsonb '"aaa"', '$.a' RETURNING char(2) OMIT QUOTES DEFAULT 'bbb' ON EMPTY);
@@ -820,12 +830,6 @@ SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING char(10));
  [1, 2]    
 (1 row)
 
-SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING char(3));
- json_query 
-------------
- [1,
-(1 row)
-
 SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING text FORMAT JSON);
  json_query 
 ------------
diff --git a/src/test/regress/sql/sqljson.sql b/src/test/regress/sql/sqljson.sql
index e6e20175b0..cbf2d6e396 100644
--- a/src/test/regress/sql/sqljson.sql
+++ b/src/test/regress/sql/sqljson.sql
@@ -463,3 +463,10 @@ SELECT '1' IS JSON AS "any", ('1' || i) IS JSON SCALAR AS "scalar", '[]' IS NOT
 \sv is_json_view
 
 DROP VIEW is_json_view;
+
+-- Test implicit coercion to a fixed-length type specified in RETURNING
+SELECT JSON_SERIALIZE('{ "a" : 1 } ' RETURNING varchar(2));
+SELECT JSON_OBJECT('a': JSON_OBJECT('b': 1 RETURNING varchar(2)));
+SELECT JSON_ARRAY(JSON_ARRAY('{ "a" : 123 }' RETURNING varchar(2)));
+SELECT JSON_ARRAYAGG(('111' || i)::bytea FORMAT JSON NULL ON NULL RETURNING varchar(2)) FROM generate_series(1,1) i;
+SELECT JSON_OBJECTAGG(i: ('111' || i)::bytea FORMAT JSON WITH UNIQUE RETURNING varchar(2)) FROM generate_series(1, 1) i;
diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql
index d9dbb1ceaa..7368cfcd92 100644
--- a/src/test/regress/sql/sqljson_queryfuncs.sql
+++ b/src/test/regress/sql/sqljson_queryfuncs.sql
@@ -53,7 +53,9 @@ SELECT JSON_VALUE(jsonb '"1.23"', '$' RETURNING int ERROR ON ERROR);
 SELECT JSON_VALUE(jsonb '"aaa"', '$');
 SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING text);
 SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING char(5));
+SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING char(2) ERROR ON ERROR);
 SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING char(2));
+SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING char(3) ERROR ON ERROR);
 SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING json);
 SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING jsonb);
 SELECT JSON_VALUE(jsonb '"aaa"', '$' RETURNING json ERROR ON ERROR);
@@ -188,8 +190,9 @@ SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING json OMIT QUOTES ERROR ON ERROR);
 SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING bytea FORMAT JSON OMIT QUOTES ERROR ON ERROR);
 
 -- Behavior when a RETURNING type has typmod != -1
-SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING char(2));
-SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING char(2) OMIT QUOTES);
+SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING char(3) ERROR ON ERROR);
+SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING char(3));
+SELECT JSON_QUERY(jsonb '"aaa"', '$' RETURNING char(3) OMIT QUOTES ERROR ON ERROR);
 SELECT JSON_QUERY(jsonb '"aaa"', '$.a' RETURNING char(2) OMIT QUOTES DEFAULT 'bbb' ON EMPTY);
 SELECT JSON_QUERY(jsonb '"aaa"', '$.a' RETURNING char(2) OMIT QUOTES DEFAULT '"bbb"'::jsonb ON EMPTY);
 
@@ -235,7 +238,6 @@ SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING jsonb);
 SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING jsonb FORMAT JSON);
 SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING text);
 SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING char(10));
-SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING char(3));
 SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING text FORMAT JSON);
 SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING bytea);
 SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING bytea FORMAT JSON);
-- 
2.43.0