From 5933a949f71a212d49f7157360786d3ba6c4b280 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Mon, 22 Jul 2024 17:14:20 +0900
Subject: [PATCH 1/2] SQL/JSON: Some fixes to JsonBehavior expression casting

1. Remove the special case handling when casting the JsonBehavior
   expressions to types with typmod, like 86d33987 did for the casting
   of SQL/JSON constructor functions.

2. Fix casting for fixed-length character and bit string types by
   using assignment-level casts.  This is again similar to what
   86d33987 did, but for ON ERROR / EMPTY expressions.

3. Use runtime coercion for the boolean ON ERROR constants so that
   the coercion works correctly for EXISTS columns that use fixed-
   length character string types.  For example, so that the default
   "false" doesn't emit an "value too long for type character(2)"
   error even when no ON ERROR clause is specified.

4. Simplify the conditions of when to use runtime coercion vs
   creating the cast expression in the parser itself.  jsonb-valued
   expressions are now always coerced at runtime and boolean
   expressions too if the target type is a string type for the
   reasons mentioned above.

5. Add a note to the document that an SQL NULL is returned if
   JsonBehavior expression cannot be coerced to the target type
   at runtime when either the ON ERROR or ON EMPTY behavior must
   be invoked.

Tests related to casting to bit(N) are taken from Jian He's patch.

Reported-by: Jian He <jian.universality@gmail.com>
Author: Jian He <jian.universality@gmail.com>
Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
---
 doc/src/sgml/func.sgml                        |   7 ++
 src/backend/parser/parse_expr.c               | 100 ++++++++++++------
 .../regress/expected/sqljson_jsontable.out    |  11 +-
 .../regress/expected/sqljson_queryfuncs.out   |  68 +++++++++---
 src/test/regress/sql/sqljson_jsontable.sql    |   5 +-
 src/test/regress/sql/sqljson_queryfuncs.sql   |  13 +++
 6 files changed, 158 insertions(+), 46 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index fd5699f4d8..a9ff8619ce 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18940,6 +18940,13 @@ DETAIL:  Missing "]" after array dimensions.
     the JSON <literal>null</literal> as is.
    </para>
   </note>
+  <note>
+   <para>
+    If an <literal>ON ERROR</literal> or <literal>ON EMPTY</literal>
+    expression can't be coerced to the <literal>RETURNING</literal> type
+    successfully, an SQL NULL value will be returned.
+   </para>
+  </note>
   </sect2>
 
  <sect2 id="functions-sqljson-table">
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 8577f27806..d31b4ffcec 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -4685,51 +4685,89 @@ transformJsonBehavior(ParseState *pstate, JsonBehavior *behavior,
 	if (expr == NULL && btype != JSON_BEHAVIOR_ERROR)
 		expr = GetJsonBehaviorConst(btype, location);
 
-	if (expr)
+	/*
+	 * Try to coerce the expression if needed.
+	 *
+	 * Use runtime coercion using json_populate_type() if the expression is
+	 * NULL, jsonb-valued, or boolean-valued.  The last only if the target is
+	 * a string type, because casting functions don't handle errors softly, so
+	 * won't correctly handle the casting of the boolean literal values to
+	 * fixed-length types correctly if they exceed the specified length.
+	 *
+	 * For other non-NULL expressions, try to find a cast and error out if
+	 * one is not found.
+	 */
+	if (expr && exprType(expr) != returning->typid)
 	{
-		Node	   *coerced_expr = expr;
 		bool		isnull = (IsA(expr, Const) && ((Const *) expr)->constisnull);
-
-		/*
-		 * Coerce NULLs and "internal" (that is, not specified by the user)
-		 * jsonb-valued expressions at runtime using json_populate_type().
-		 *
-		 * For other (user-specified) non-NULL values, try to find a cast and
-		 * error out if one is not found.
-		 */
+		char		typcategory = TypeCategory(returning->typid);
 		if (isnull ||
-			(exprType(expr) == JSONBOID &&
-			 btype == default_behavior))
+			exprType(expr) == JSONBOID ||
+			(exprType(expr) == BOOLOID && typcategory == TYPCATEGORY_STRING))
+		{
 			coerce_at_runtime = true;
+
+			/*
+			 * json_populate_type() expects to be passed a jsonb value, so
+			 * gin up a Const containing the appropriate boolean value
+			 * represented as jsonb, discarding the original Const containing a
+			 * plain boolean.
+			 */
+			if (exprType(expr) == BOOLOID)
+			{
+				char   *val = btype == JSON_BEHAVIOR_TRUE ? "true" : "false";
+
+				expr = (Node *) makeConst(JSONBOID, -1, InvalidOid, -1,
+										  DirectFunctionCall1(jsonb_in,
+															  CStringGetDatum(val)),
+										  false,false);
+			}
+		}
 		else
 		{
-			int32		baseTypmod = returning->typmod;
+			Node	*coerced_expr;
 
-			if (get_typtype(returning->typid) == TYPTYPE_DOMAIN)
-				(void) getBaseTypeAndTypmod(returning->typid, &baseTypmod);
-
-			if (baseTypmod > 0)
-				expr = coerce_to_specific_type(pstate, expr, TEXTOID,
-											   "JSON_FUNCTION()");
+			/*
+			 * Use an assignment cast if coercing to a string type so that
+			 * build_coercion_expression() assumes implicit coercion when
+			 * coercing the typmod, so that inputs exceeding length cause an
+			 * error instead of silent truncation.
+			 */
 			coerced_expr =
 				coerce_to_target_type(pstate, expr, exprType(expr),
-									  returning->typid, baseTypmod,
-									  baseTypmod > 0 ? COERCION_IMPLICIT :
+									  returning->typid, returning->typmod,
+									  (typcategory == TYPCATEGORY_STRING ||
+									   typcategory == TYPCATEGORY_BITSTRING) ?
+									  COERCION_ASSIGNMENT :
 									  COERCION_EXPLICIT,
-									  baseTypmod > 0 ? COERCE_IMPLICIT_CAST :
 									  COERCE_EXPLICIT_CAST,
 									  exprLocation((Node *) behavior));
-		}
 
-		if (coerced_expr == NULL)
-			ereport(ERROR,
-					errcode(ERRCODE_CANNOT_COERCE),
-					errmsg("cannot cast behavior expression of type %s to %s",
-						   format_type_be(exprType(expr)),
-						   format_type_be(returning->typid)),
-					parser_errposition(pstate, exprLocation(expr)));
-		else
+			if (coerced_expr == NULL)
+			{
+				/*
+				 * Provide a HINT if the expression comes from a DEFAULT
+				 * clause.
+				 */
+				if (btype == JSON_BEHAVIOR_DEFAULT)
+					ereport(ERROR,
+							errcode(ERRCODE_CANNOT_COERCE),
+							errmsg("cannot cast behavior expression of type %s to %s",
+								   format_type_be(exprType(expr)),
+								   format_type_be(returning->typid)),
+							errhint("You will need to cast the expression."),
+							parser_errposition(pstate, exprLocation(expr)));
+				else
+					ereport(ERROR,
+							errcode(ERRCODE_CANNOT_COERCE),
+							errmsg("cannot cast behavior expression of type %s to %s",
+								   format_type_be(exprType(expr)),
+								   format_type_be(returning->typid)),
+							parser_errposition(pstate, exprLocation(expr)));
+			}
+
 			expr = coerced_expr;
+		}
 	}
 
 	if (behavior)
diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out
index 5fd43be367..939e1b2f87 100644
--- a/src/test/regress/expected/sqljson_jsontable.out
+++ b/src/test/regress/expected/sqljson_jsontable.out
@@ -560,7 +560,16 @@ SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int8 EXISTS PATH '$.a'));
 ERROR:  cannot cast behavior expression of type boolean to bigint
 SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a float4 EXISTS PATH '$.a'));
 ERROR:  cannot cast behavior expression of type boolean to real
-SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a char(5) EXISTS PATH '$.a'));
+-- Default FALSE (ON ERROR) doesn't fit char(3)
+SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a char(3) EXISTS PATH '$.a'));
+ a 
+---
+ 
+(1 row)
+
+SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a char(3) EXISTS PATH '$.a' ERROR ON ERROR));
+ERROR:  value too long for type character(3)
+SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a char(5) EXISTS PATH '$.a' ERROR ON ERROR));
    a   
 -------
  false
diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index 074aedb2dd..ef5dfb4157 100644
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -665,7 +665,11 @@ SELECT JSON_QUERY(jsonb '"aaa"', '$.a' RETURNING char(2) OMIT QUOTES DEFAULT 'bb
 (1 row)
 
 SELECT JSON_QUERY(jsonb '"aaa"', '$.a' RETURNING char(2) OMIT QUOTES DEFAULT '"bb"'::jsonb ON EMPTY);
-ERROR:  value too long for type character(2)
+ json_query 
+------------
+ bb
+(1 row)
+
 -- OMIT QUOTES behavior should not be specified when WITH WRAPPER used:
 -- Should fail
 SELECT JSON_QUERY(jsonb '[1]', '$' WITH WRAPPER OMIT QUOTES);
@@ -853,13 +857,17 @@ SELECT JSON_QUERY(jsonb '[1,2]', '$' RETURNING bytea FORMAT JSON);
 (1 row)
 
 SELECT JSON_QUERY(jsonb '[1,2]', '$[*]' RETURNING bytea EMPTY OBJECT ON ERROR);
-ERROR:  cannot cast behavior expression of type jsonb to bytea
-LINE 1: ... JSON_QUERY(jsonb '[1,2]', '$[*]' RETURNING bytea EMPTY OBJE...
-                                                             ^
+ json_query 
+------------
+ \x7b7d
+(1 row)
+
 SELECT JSON_QUERY(jsonb '[1,2]', '$[*]' RETURNING bytea FORMAT JSON EMPTY OBJECT ON ERROR);
-ERROR:  cannot cast behavior expression of type jsonb to bytea
-LINE 1: ...jsonb '[1,2]', '$[*]' RETURNING bytea FORMAT JSON EMPTY OBJE...
-                                                             ^
+ json_query 
+------------
+ \x7b7d
+(1 row)
+
 SELECT JSON_QUERY(jsonb '[1,2]', '$[*]' RETURNING json EMPTY OBJECT ON ERROR);
  json_query 
 ------------
@@ -873,13 +881,17 @@ SELECT JSON_QUERY(jsonb '[1,2]', '$[*]' RETURNING jsonb EMPTY OBJECT ON ERROR);
 (1 row)
 
 SELECT JSON_QUERY(jsonb '[3,4]', '$[*]' RETURNING bigint[] EMPTY OBJECT ON ERROR);
-ERROR:  cannot cast behavior expression of type jsonb to bigint[]
-LINE 1: ...ON_QUERY(jsonb '[3,4]', '$[*]' RETURNING bigint[] EMPTY OBJE...
-                                                             ^
+ json_query 
+------------
+ 
+(1 row)
+
 SELECT JSON_QUERY(jsonb '"[3,4]"', '$[*]' RETURNING bigint[] EMPTY OBJECT ON ERROR);
-ERROR:  cannot cast behavior expression of type jsonb to bigint[]
-LINE 1: ..._QUERY(jsonb '"[3,4]"', '$[*]' RETURNING bigint[] EMPTY OBJE...
-                                                             ^
+ json_query 
+------------
+ 
+(1 row)
+
 -- Coercion fails with quotes on
 SELECT JSON_QUERY(jsonb '"123.1"', '$' RETURNING int2 error on error);
 ERROR:  invalid input syntax for type smallint: ""123.1""
@@ -1405,3 +1417,33 @@ SELECT JSON_VALUE(jsonb '123', '$' RETURNING queryfuncs_char2_chk DEFAULT 1 ON E
 (1 row)
 
 DROP DOMAIN queryfuncs_char2, queryfuncs_char2_chk;
+-- Test coercion to domain over another fixed-legth type of the ON ERROR /
+-- EMPTY expressions.  Ask user to cast the DEFAULT expression explicitly if
+-- automatic casting cannot be done, for example, from int to bit(2).
+CREATE DOMAIN queryfuncs_d_varbit3 AS varbit(3) CHECK (VALUE <> '01');
+SELECT JSON_VALUE(jsonb '1234', '$' RETURNING queryfuncs_d_varbit3  DEFAULT '111111' ON ERROR);
+ERROR:  bit string too long for type bit varying(3)
+SELECT JSON_VALUE(jsonb '1234', '$' RETURNING queryfuncs_d_varbit3  DEFAULT '010' ON ERROR);
+ json_value 
+------------
+ 010
+(1 row)
+
+SELECT JSON_VALUE(jsonb '1234', '$' RETURNING queryfuncs_d_varbit3  DEFAULT '01' ON ERROR);
+ERROR:  value for domain queryfuncs_d_varbit3 violates check constraint "queryfuncs_d_varbit3_check"
+SELECT JSON_VALUE(jsonb '"111"', '$'  RETURNING bit(2) ERROR ON ERROR);
+ERROR:  bit string length 3 does not match type bit(2)
+SELECT JSON_VALUE(jsonb '1234', '$' RETURNING bit(3)  DEFAULT 1 ON ERROR);
+ERROR:  cannot cast behavior expression of type integer to bit
+LINE 1: ...VALUE(jsonb '1234', '$' RETURNING bit(3)  DEFAULT 1 ON ERROR...
+                                                             ^
+HINT:  You will need to cast the expression.
+SELECT JSON_VALUE(jsonb '1234', '$' RETURNING bit(3)  DEFAULT 1::bit(3) ON ERROR);
+ json_value 
+------------
+ 001
+(1 row)
+
+SELECT JSON_VALUE(jsonb '"111"', '$.a'  RETURNING bit(3) DEFAULT '1111' ON EMPTY);
+ERROR:  bit string length 4 does not match type bit(3)
+DROP DOMAIN queryfuncs_d_varbit3;
diff --git a/src/test/regress/sql/sqljson_jsontable.sql b/src/test/regress/sql/sqljson_jsontable.sql
index 4594e5b013..dc509bed8a 100644
--- a/src/test/regress/sql/sqljson_jsontable.sql
+++ b/src/test/regress/sql/sqljson_jsontable.sql
@@ -266,7 +266,10 @@ SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int4 EXISTS PATH '$.a'));
 SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int2 EXISTS PATH '$.a'));
 SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int8 EXISTS PATH '$.a'));
 SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a float4 EXISTS PATH '$.a'));
-SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a char(5) EXISTS PATH '$.a'));
+-- Default FALSE (ON ERROR) doesn't fit char(3)
+SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a char(3) EXISTS PATH '$.a'));
+SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a char(3) EXISTS PATH '$.a' ERROR ON ERROR));
+SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a char(5) EXISTS PATH '$.a' ERROR ON ERROR));
 SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a json EXISTS PATH '$.a'));
 SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a jsonb EXISTS PATH '$.a'));
 
diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql
index be5593b332..6875891843 100644
--- a/src/test/regress/sql/sqljson_queryfuncs.sql
+++ b/src/test/regress/sql/sqljson_queryfuncs.sql
@@ -479,3 +479,16 @@ SELECT JSON_VALUE(jsonb '123', '$' RETURNING queryfuncs_char2 DEFAULT 1 ON ERROR
 SELECT JSON_VALUE(jsonb '123', '$' RETURNING queryfuncs_char2_chk ERROR ON ERROR);
 SELECT JSON_VALUE(jsonb '123', '$' RETURNING queryfuncs_char2_chk DEFAULT 1 ON ERROR);
 DROP DOMAIN queryfuncs_char2, queryfuncs_char2_chk;
+
+-- Test coercion to domain over another fixed-legth type of the ON ERROR /
+-- EMPTY expressions.  Ask user to cast the DEFAULT expression explicitly if
+-- automatic casting cannot be done, for example, from int to bit(2).
+CREATE DOMAIN queryfuncs_d_varbit3 AS varbit(3) CHECK (VALUE <> '01');
+SELECT JSON_VALUE(jsonb '1234', '$' RETURNING queryfuncs_d_varbit3  DEFAULT '111111' ON ERROR);
+SELECT JSON_VALUE(jsonb '1234', '$' RETURNING queryfuncs_d_varbit3  DEFAULT '010' ON ERROR);
+SELECT JSON_VALUE(jsonb '1234', '$' RETURNING queryfuncs_d_varbit3  DEFAULT '01' ON ERROR);
+SELECT JSON_VALUE(jsonb '"111"', '$'  RETURNING bit(2) ERROR ON ERROR);
+SELECT JSON_VALUE(jsonb '1234', '$' RETURNING bit(3)  DEFAULT 1 ON ERROR);
+SELECT JSON_VALUE(jsonb '1234', '$' RETURNING bit(3)  DEFAULT 1::bit(3) ON ERROR);
+SELECT JSON_VALUE(jsonb '"111"', '$.a'  RETURNING bit(3) DEFAULT '1111' ON EMPTY);
+DROP DOMAIN queryfuncs_d_varbit3;
-- 
2.43.0

