From 0a8b756a56f3750ad07f09d7933c9cae0218ec93 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Sat, 22 Jun 2024 17:28:53 +0800
Subject: [PATCH v2 2/2] Disallow incompatible values in ON ERROR/EMPTY

make json_exists only allow
     { TRUE | FALSE | UNKNOWN | ERROR } ON ERROR

make json_query, json_value only allow
    { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression } ON EMPTY
    { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression } ON ERROR

make json_value, json_query on empty, on error behave the same way,
because for scalar jsonb, json_value, json_query behave the same.
so sometimes they may need to return the same value.

also json_table function may call json_value or json_query,
make these two function on error, on empty behavior the same can reduce unintended complex.
---
 src/backend/parser/parse_expr.c               | 143 ++++++++++--------
 .../regress/expected/sqljson_jsontable.out    |  30 ++++
 .../regress/expected/sqljson_queryfuncs.out   |  11 +-
 src/test/regress/sql/sqljson_jsontable.sql    |   7 +
 src/test/regress/sql/sqljson_queryfuncs.sql   |   2 +-
 5 files changed, 126 insertions(+), 67 deletions(-)

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 3d79d171..f3ffa88f 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -4299,74 +4299,95 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func)
 					parser_errposition(pstate, format->location));
 	}
 
-	/* OMIT QUOTES is meaningless when strings are wrapped. */
+	/* check json_query, json_value, on error, on empty behavior */
+	if (func->op == JSON_QUERY_OP || func->op == JSON_VALUE_OP)
+	{
+		if (func->on_empty != NULL &&
+		   (func->on_empty->btype == JSON_BEHAVIOR_TRUE ||
+			func->on_empty->btype == JSON_BEHAVIOR_FALSE ||
+			func->on_empty->btype == JSON_BEHAVIOR_UNKNOWN))
+		{
+			if (func->column_name)
+				ereport(ERROR,
+						errcode(ERRCODE_SYNTAX_ERROR),
+						errmsg("invalid ON EMPTY behavior"),
+						errdetail("Only ERROR, NULL, EMPTY, EMPTY ARRAY, EMPTY OBJECT, or DEFAULT expression is allowed in ON EMPTY for column \"%s\".",
+								   func->column_name),
+						parser_errposition(pstate, func->on_empty->location));
+			else
+				ereport(ERROR,
+						errcode(ERRCODE_SYNTAX_ERROR),
+						errmsg("invalid ON EMPTY behavior"),
+						errdetail("Only ERROR, NULL, EMPTY, EMPTY ARRAY, EMPTY OBJECT, or DEFAULT expression is allowed in ON EMPTY for %s().",
+								   func_name),
+						parser_errposition(pstate, func->on_empty->location));
+		}
+
+		if (func->on_error != NULL &&
+		   (func->on_error->btype == JSON_BEHAVIOR_TRUE ||
+			func->on_error->btype == JSON_BEHAVIOR_FALSE ||
+			func->on_error->btype == JSON_BEHAVIOR_UNKNOWN))
+		{
+			if (func->column_name)
+				ereport(ERROR,
+						errcode(ERRCODE_SYNTAX_ERROR),
+						errmsg("invalid ON ERROR behavior"),
+						errdetail("Only ERROR, NULL, EMPTY, EMPTY ARRAY, EMPTY OBJECT, or DEFAULT expression is allowed in ON ERROR for column \"%s\".",
+								   func->column_name),
+						parser_errposition(pstate, func->on_error->location));
+			else
+				ereport(ERROR,
+						errcode(ERRCODE_SYNTAX_ERROR),
+						errmsg("invalid ON ERROR behavior"),
+						errdetail("Only ERROR, NULL, EMPTY, EMPTY ARRAY, EMPTY OBJECT, or DEFAULT expression is allowed in ON ERROR for %s().",
+								   func_name),
+						parser_errposition(pstate, func->on_error->location));
+		}
+	}
+
+	/* Check that ON ERROR/EMPTY behavior values are valid for json_exists. */
+	if (func->op == JSON_EXISTS_OP)
+	{
+		if (func->column_name && func->on_empty != NULL)
+			ereport(ERROR,
+					errcode(ERRCODE_SYNTAX_ERROR),
+					errmsg("invalid ON EMPTY behavior"),
+					errdetail("ON EMPTY is not allowed for column \"%s\".",
+							   func->column_name),
+					parser_errposition(pstate, func->on_empty->location));
+
+  		if (func->on_error != NULL &&
+			func->on_error->btype != JSON_BEHAVIOR_ERROR &&
+			func->on_error->btype != JSON_BEHAVIOR_TRUE &&
+			func->on_error->btype != JSON_BEHAVIOR_FALSE &&
+			func->on_error->btype != JSON_BEHAVIOR_UNKNOWN)
+		{
+			if (!func->column_name)
+				ereport(ERROR,
+						errcode(ERRCODE_SYNTAX_ERROR),
+						errmsg("invalid ON ERROR behavior"),
+						errdetail("Only ERROR, TRUE, FALSE, or UNKNOWN is allowed in ON ERROR for JSON_EXISTS()."),
+						parser_errposition(pstate, func->on_error->location));
+			else
+				ereport(ERROR,
+						errcode(ERRCODE_SYNTAX_ERROR),
+						errmsg("invalid ON ERROR behavior"),
+						errdetail("Only ERROR, TRUE, FALSE, or UNKNOWN is allowed in ON ERROR for column \"%s\".",
+								   func->column_name),
+						parser_errposition(pstate, func->on_error->location));
+		}
+	}
+
 	if (func->op == JSON_QUERY_OP)
 	{
+		/* OMIT QUOTES is meaningless when strings are wrapped. */
 		if (func->quotes == JS_QUOTES_OMIT &&
-			(func->wrapper == JSW_CONDITIONAL ||
-			 func->wrapper == JSW_UNCONDITIONAL))
+		   (func->wrapper == JSW_CONDITIONAL ||
+		 	func->wrapper == JSW_UNCONDITIONAL))
 			ereport(ERROR,
 					errcode(ERRCODE_SYNTAX_ERROR),
 					errmsg("SQL/JSON QUOTES behavior must not be specified when WITH WRAPPER is used"),
 					parser_errposition(pstate, func->location));
-		if (func->on_empty != NULL &&
-			func->on_empty->btype != JSON_BEHAVIOR_ERROR &&
-			func->on_empty->btype != JSON_BEHAVIOR_NULL &&
-			func->on_empty->btype != JSON_BEHAVIOR_EMPTY &&
-			func->on_empty->btype != JSON_BEHAVIOR_EMPTY_ARRAY &&
-			func->on_empty->btype != JSON_BEHAVIOR_EMPTY_OBJECT &&
-			func->on_empty->btype != JSON_BEHAVIOR_DEFAULT)
-			ereport(ERROR,
-					errcode(ERRCODE_SYNTAX_ERROR),
-					errmsg("invalid ON EMPTY behavior"),
-					errdetail("Only ERROR, NULL, EMPTY [ ARRAY | OBJECT }, or DEFAULT <value> is allowed in ON EMPTY for JSON_QUERY()."),
-					parser_errposition(pstate, func->on_empty->location));
-		if (func->on_error != NULL &&
-			func->on_error->btype != JSON_BEHAVIOR_ERROR &&
-			func->on_error->btype != JSON_BEHAVIOR_NULL &&
-			func->on_error->btype != JSON_BEHAVIOR_EMPTY &&
-			func->on_error->btype != JSON_BEHAVIOR_EMPTY_ARRAY &&
-			func->on_error->btype != JSON_BEHAVIOR_EMPTY_OBJECT &&
-			func->on_error->btype != JSON_BEHAVIOR_DEFAULT)
-			ereport(ERROR,
-					errcode(ERRCODE_SYNTAX_ERROR),
-					errmsg("invalid ON ERROR behavior"),
-					errdetail("Only ERROR, NULL, EMPTY [ ARRAY | OBJECT }, or DEFAULT <value> is allowed in ON ERROR for JSON_QUERY()."),
-					parser_errposition(pstate, func->on_error->location));
-	}
-
-	/* Check that ON ERROR/EMPTY behavior values are valid for the function. */
-	if (func->op == JSON_EXISTS_OP &&
-		func->on_error != NULL &&
-		func->on_error->btype != JSON_BEHAVIOR_ERROR &&
-		func->on_error->btype != JSON_BEHAVIOR_TRUE &&
-		func->on_error->btype != JSON_BEHAVIOR_FALSE &&
-		func->on_error->btype != JSON_BEHAVIOR_UNKNOWN)
-		ereport(ERROR,
-				errcode(ERRCODE_SYNTAX_ERROR),
-				errmsg("invalid ON ERROR behavior"),
-				errdetail("Only ERROR, TRUE, FALSE, or UNKNOWN is allowed in ON ERROR for JSON_EXISTS()."),
-				parser_errposition(pstate, func->on_error->location));
-	if (func->op == JSON_VALUE_OP)
-	{
-		if (func->on_empty != NULL &&
-			func->on_empty->btype != JSON_BEHAVIOR_ERROR &&
-			func->on_empty->btype != JSON_BEHAVIOR_NULL &&
-			func->on_empty->btype != JSON_BEHAVIOR_DEFAULT)
-			ereport(ERROR,
-					errcode(ERRCODE_SYNTAX_ERROR),
-					errmsg("invalid ON ERROR behavior"),
-					errdetail("Only ERROR, NULL, or DEFAULT <value> is allowed in ON EMPTY for JSON_VALUE()."),
-					parser_errposition(pstate, func->on_empty->location));
-		if (func->on_error != NULL &&
-			func->on_error->btype != JSON_BEHAVIOR_ERROR &&
-			 func->on_error->btype != JSON_BEHAVIOR_NULL &&
-			 func->on_error->btype != JSON_BEHAVIOR_DEFAULT)
-			ereport(ERROR,
-					errcode(ERRCODE_SYNTAX_ERROR),
-					errmsg("invalid ON EMPTY behavior"),
-					errdetail("Only ERROR, NULL, or DEFAULT <value> is allowed in ON ERROR for JSON_VALUE()."),
-					parser_errposition(pstate, func->on_error->location));
 	}
 
 	jsexpr = makeNode(JsonExpr);
diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out
index eace29ef..8d372764 100644
--- a/src/test/regress/expected/sqljson_jsontable.out
+++ b/src/test/regress/expected/sqljson_jsontable.out
@@ -1073,3 +1073,33 @@ ERROR:  invalid ON ERROR behavior
 LINE 1: ... * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int) NULL ON ER...
                                                              ^
 DETAIL:  Only EMPTY or ERROR is allowed in the top-level ON ERROR clause.
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int true on empty));
+ERROR:  invalid ON EMPTY behavior
+LINE 1: ...T * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int true on em...
+                                                             ^
+DETAIL:  Only ERROR, NULL, EMPTY, EMPTY ARRAY, EMPTY OBJECT, or DEFAULT expression is allowed in ON EMPTY for column "a".
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int omit quotes true on error));
+ERROR:  invalid ON ERROR behavior
+LINE 1: ...N_TABLE(jsonb '1', '$' COLUMNS (a int omit quotes true on er...
+                                                             ^
+DETAIL:  Only ERROR, NULL, EMPTY, EMPTY ARRAY, EMPTY OBJECT, or DEFAULT expression is allowed in ON ERROR for column "a".
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int keep quotes empty on error));
+ERROR:  cannot cast jsonb array to type integer
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int keep quotes empty object on error));
+ERROR:  cannot cast jsonb object to type integer
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a jsonb keep quotes empty object on empty)); --ok
+ a  
+----
+ {}
+(1 row)
+
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int exists empty object on error));
+ERROR:  invalid ON ERROR behavior
+LINE 1: ...M JSON_TABLE(jsonb '1', '$' COLUMNS (a int exists empty obje...
+                                                             ^
+DETAIL:  Only ERROR, TRUE, FALSE, or UNKNOWN is allowed in ON ERROR for column "a".
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int exists true on empty));
+ERROR:  invalid ON EMPTY behavior
+LINE 1: ...M JSON_TABLE(jsonb '1', '$' COLUMNS (a int exists true on em...
+                                                             ^
+DETAIL:  ON EMPTY is not allowed for column "a".
diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index 0c4b62b0..37bb5ad7 100644
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -1360,12 +1360,13 @@ LINE 1: SELECT JSON_EXISTS(jsonb '1', '$' DEFAULT 1 ON ERROR);
                                           ^
 DETAIL:  Only ERROR, TRUE, FALSE, or UNKNOWN is allowed in ON ERROR for JSON_EXISTS().
 SELECT JSON_VALUE(jsonb '1', '$' EMPTY ON ERROR);
-ERROR:  invalid ON EMPTY behavior
-LINE 1: SELECT JSON_VALUE(jsonb '1', '$' EMPTY ON ERROR);
-                                         ^
-DETAIL:  Only ERROR, NULL, or DEFAULT <value> is allowed in ON ERROR for JSON_VALUE().
+ json_value 
+------------
+ 1
+(1 row)
+
 SELECT JSON_QUERY(jsonb '1', '$' TRUE ON ERROR);
 ERROR:  invalid ON ERROR behavior
 LINE 1: SELECT JSON_QUERY(jsonb '1', '$' TRUE ON ERROR);
                                          ^
-DETAIL:  Only ERROR, NULL, EMPTY [ ARRAY | OBJECT }, or DEFAULT <value> is allowed in ON ERROR for JSON_QUERY().
+DETAIL:  Only ERROR, NULL, EMPTY, EMPTY ARRAY, EMPTY OBJECT, or DEFAULT expression is allowed in ON ERROR for JSON_QUERY().
diff --git a/src/test/regress/sql/sqljson_jsontable.sql b/src/test/regress/sql/sqljson_jsontable.sql
index 1f81464c..3bb49ba9 100644
--- a/src/test/regress/sql/sqljson_jsontable.sql
+++ b/src/test/regress/sql/sqljson_jsontable.sql
@@ -521,3 +521,10 @@ DROP TABLE s;
 
 -- Test ON ERROR / EMPTY value validity for the function
 SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int) NULL ON ERROR);	-- fail
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int true on empty));
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int omit quotes true on error));
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int keep quotes empty on error));
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int keep quotes empty object on error));
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a jsonb keep quotes empty object on empty)); --ok
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int exists empty object on error));
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int exists true on empty));
diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql
index 4586fdb8..0f7b6ec7 100644
--- a/src/test/regress/sql/sqljson_queryfuncs.sql
+++ b/src/test/regress/sql/sqljson_queryfuncs.sql
@@ -462,5 +462,5 @@ SELECT JSON_QUERY(jsonb 'null', '$xyz' PASSING 1 AS xyz);
 
 -- Test ON ERROR / EMPTY value validity for the function; all fail.
 SELECT JSON_EXISTS(jsonb '1', '$' DEFAULT 1 ON ERROR);
-SELECT JSON_VALUE(jsonb '1', '$' EMPTY ON ERROR);
+SELECT JSON_VALUE(jsonb '1', '$' UNKNOWN ON ERROR);
 SELECT JSON_QUERY(jsonb '1', '$' TRUE ON ERROR);
-- 
2.34.1

