From edcef782abd801f5e52de6fed364fe9db92717e3 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Thu, 18 Apr 2024 09:21:33 +0900
Subject: [PATCH v5 3/3] SQL/JSON: Fix issues with DEFAULT .. ON ERROR / EMPTY

SQL/JSON query functions allow specifying an expression to return
when either of ON ERROR or ON EMPTY condition occurs when evaluating
the JSON path expression.  The parser (transformJsonBehavior()) checks
that the specified expression is one of the supported expressions, but
there are two issues with how the check is done that are fixed in this
commit:

* No check for some expressions related to coercion, such as
  CoerceViaIO, that may appear in the transformed user-specified
  expressions that include cast(s)

* An unsupported expression may be masked by a coercion-related
  expression, which must be flagged by checking the latter's
  argument expression recursively

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/CACJufxEqhqsfrg_p7EMyo5zak3d767iFDL8vz_4%3DZBHpOtrghw@mail.gmail.com
Discussion: https://postgr.es/m/CACJufxGOerH1QJknm1noh-Kz5FqU4p7QfeZSeVT2tN_4SLXYNg@mail.gmail.com
---
 src/backend/parser/parse_expr.c               | 37 +++++++++++-
 .../regress/expected/sqljson_jsontable.out    | 32 +++++++++++
 .../regress/expected/sqljson_queryfuncs.out   | 57 +++++++++++++++++++
 src/test/regress/sql/sqljson_jsontable.sql    | 16 ++++++
 src/test/regress/sql/sqljson_queryfuncs.sql   | 27 +++++++++
 5 files changed, 167 insertions(+), 2 deletions(-)

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 34ac17868b..02bc37b38c 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -4616,6 +4616,40 @@ coerceJsonExprOutput(ParseState *pstate, JsonExpr *jsexpr)
 		   (jsexpr->use_io_coercion != jsexpr->use_json_coercion));
 }
 
+/*
+ * Recursively checks if the given expression, or its sub-node in some cases,
+ * is valid for using as an ON ERROR / ON EMPTY DEFAULT expression.
+ */
+static bool
+ValidJsonBehaviorDefaultExpr(Node *expr, void *context)
+{
+	if (expr == NULL)
+		return false;
+
+	switch(nodeTag(expr))
+	{
+		/* Acceptable expression nodes */
+		case T_Const:
+		case T_FuncExpr:
+		case T_OpExpr:
+			return true;
+
+		/* Acceptable iff arg of the following nodes is one of the above */
+		case T_CoerceViaIO:
+		case T_CoerceToDomain:
+		case T_ArrayCoerceExpr:
+		case T_ConvertRowtypeExpr:
+		case T_RelabelType:
+		case T_CollateExpr:
+			return expression_tree_walker(expr, ValidJsonBehaviorDefaultExpr,
+										  context);
+		default:
+			break;
+	}
+
+	return false;
+}
+
 /*
  * Transform a JSON BEHAVIOR clause.
  */
@@ -4636,8 +4670,7 @@ transformJsonBehavior(ParseState *pstate, JsonBehavior *behavior,
 		if (btype == JSON_BEHAVIOR_DEFAULT)
 		{
 			expr = transformExprRecurse(pstate, behavior->expr);
-			if (!IsA(expr, Const) && !IsA(expr, FuncExpr) &&
-				!IsA(expr, OpExpr))
+			if (!ValidJsonBehaviorDefaultExpr(expr, NULL))
 				ereport(ERROR,
 						(errcode(ERRCODE_DATATYPE_MISMATCH),
 						 errmsg("can only specify a constant, non-aggregate function, or operator expression for DEFAULT"),
diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out
index 9eecd97f45..af1ed09f9f 100644
--- a/src/test/regress/expected/sqljson_jsontable.out
+++ b/src/test/regress/expected/sqljson_jsontable.out
@@ -217,6 +217,38 @@ FROM json_table_test vals
  [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 11 |     |      | "\"str\""    | ["\"str\""]    | "str"        |    |    | 
 (14 rows)
 
+-- Test using casts in DEFAULT .. ON ERROR expression
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
+    COLUMNS (js1 jsonb_test_domain path '$.a2' DEFAULT '"foo1"'::jsonb::text ON ERROR));
+  js1   
+--------
+ "foo1"
+(1 row)
+
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
+    COLUMNS (js1 jsonb_test_domain path '$.a2' DEFAULT 'foo'::jsonb_test_domain ON ERROR));
+ERROR:  value for domain jsonb_test_domain violates check constraint "jsonb_test_domain_check"
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
+    COLUMNS (js1 jsonb_test_domain path '$.a2' DEFAULT 'foo1'::jsonb_test_domain ON ERROR));
+ js1  
+------
+ foo1
+(1 row)
+
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "foo"}', '$'
+    COLUMNS (js1 jsonb_test_domain path '$.d1' DEFAULT 'foo2'::jsonb_test_domain ON ERROR));
+ js1  
+------
+ foo2
+(1 row)
+
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "foo"}', '$'
+    COLUMNS (js1 oid[] path '$.d2' DEFAULT '{1}'::int[]::oid[] ON ERROR));
+ js1 
+-----
+ {1}
+(1 row)
+
 -- JSON_TABLE: Test backward parsing
 CREATE VIEW jsonb_table_view2 AS
 SELECT * FROM
diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index 49b014b1ec..003b7fead6 100644
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -1222,6 +1222,63 @@ LINE 1: SELECT JSON_QUERY(js, '$'  RETURNING int DEFAULT (SELECT 1) ...
                                                          ^
 DROP TABLE test_jsonb_mutability;
 DROP FUNCTION ret_setint;
+CREATE DOMAIN jsonb_test_domain AS text CHECK (value <> 'foo');
+SELECT JSON_VALUE(jsonb '{"d1": "H"}', '$.a2' RETURNING jsonb_test_domain DEFAULT 'foo'::jsonb_test_domain ON ERROR);
+ERROR:  value for domain jsonb_test_domain violates check constraint "jsonb_test_domain_check"
+SELECT JSON_VALUE(jsonb '{"d1": "H"}', '$.a2' RETURNING jsonb_test_domain DEFAULT 'foo1'::jsonb_test_domain ON ERROR);
+ json_value 
+------------
+ foo1
+(1 row)
+
+SELECT JSON_VALUE(jsonb '{"d1": "H"}', '$.a2' RETURNING jsonb_test_domain DEFAULT '"foo1"'::jsonb::text ON ERROR);
+ json_value 
+------------
+ "foo1"
+(1 row)
+
+SELECT JSON_VALUE(jsonb '{"d1": "foo"}', '$.a2' RETURNING jsonb_test_domain DEFAULT 'foo1'::jsonb_test_domain ON ERROR);
+ json_value 
+------------
+ foo1
+(1 row)
+
+-- Check the cases where a coercion-related expression is masking an
+-- unsupported expressions
+-- CoerceViaIO
+SELECT JSON_QUERY('"a"', '$.a'  RETURNING int DEFAULT (SELECT '"1"')::jsonb ON ERROR);
+ERROR:  can only specify a constant, non-aggregate function, or operator expression for DEFAULT
+LINE 1: ...CT JSON_QUERY('"a"', '$.a'  RETURNING int DEFAULT (SELECT '"...
+                                                             ^
+-- CoerceToDomain
+SELECT JSON_QUERY('"a"', '$.a' RETURNING jsonb_test_domain DEFAULT (select '"1"')::jsonb_test_domain ON ERROR);
+ERROR:  can only specify a constant, non-aggregate function, or operator expression for DEFAULT
+LINE 1: ...('"a"', '$.a' RETURNING jsonb_test_domain DEFAULT (select '"...
+                                                             ^
+-- RelabelType
+SELECT JSON_QUERY('"a"', '$.a'  RETURNING int DEFAULT (SELECT 1)::oid::int ON ERROR);
+ERROR:  can only specify a constant, non-aggregate function, or operator expression for DEFAULT
+LINE 1: ...CT JSON_QUERY('"a"', '$.a'  RETURNING int DEFAULT (SELECT 1)...
+                                                             ^
+-- ArrayCoerceExpr
+SELECT JSON_QUERY('"a"', '$.a'  RETURNING int[] DEFAULT (SELECT '{1}')::oid[]::int[] ON ERROR);
+ERROR:  can only specify a constant, non-aggregate function, or operator expression for DEFAULT
+LINE 1: ... JSON_QUERY('"a"', '$.a'  RETURNING int[] DEFAULT (SELECT '{...
+                                                             ^
+-- CollateExpr
+SELECT JSON_QUERY('"a"', '$.a'  RETURNING int[] DEFAULT (SELECT '{1}')::text COLLATE "C" ON ERROR);
+ERROR:  can only specify a constant, non-aggregate function, or operator expression for DEFAULT
+LINE 1: ... JSON_QUERY('"a"', '$.a'  RETURNING int[] DEFAULT (SELECT '{...
+                                                             ^
+-- ConvertRowtypeExpr
+CREATE TABLE someparent (a int);
+CREATE TABLE somechild () INHERITS (someparent);
+SELECT JSON_QUERY('"a"', '$.a'  RETURNING someparent DEFAULT (SELECT '(1)')::somechild::someparent ON ERROR);
+ERROR:  can only specify a constant, non-aggregate function, or operator expression for DEFAULT
+LINE 1: ..._QUERY('"a"', '$.a'  RETURNING someparent DEFAULT (SELECT '(...
+                                                             ^
+DROP DOMAIN jsonb_test_domain;
+DROP TABLE someparent, somechild;
 -- Extension: non-constant JSON path
 SELECT JSON_EXISTS(jsonb '{"a": 123}', '$' || '.' || 'a');
  json_exists 
diff --git a/src/test/regress/sql/sqljson_jsontable.sql b/src/test/regress/sql/sqljson_jsontable.sql
index 29c0c6ba52..670c0f69d9 100644
--- a/src/test/regress/sql/sqljson_jsontable.sql
+++ b/src/test/regress/sql/sqljson_jsontable.sql
@@ -116,6 +116,22 @@ FROM json_table_test vals
 	) jt
 	ON true;
 
+-- Test using casts in DEFAULT .. ON ERROR expression
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
+    COLUMNS (js1 jsonb_test_domain path '$.a2' DEFAULT '"foo1"'::jsonb::text ON ERROR));
+
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
+    COLUMNS (js1 jsonb_test_domain path '$.a2' DEFAULT 'foo'::jsonb_test_domain ON ERROR));
+
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
+    COLUMNS (js1 jsonb_test_domain path '$.a2' DEFAULT 'foo1'::jsonb_test_domain ON ERROR));
+
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "foo"}', '$'
+    COLUMNS (js1 jsonb_test_domain path '$.d1' DEFAULT 'foo2'::jsonb_test_domain ON ERROR));
+
+SELECT * FROM JSON_TABLE(jsonb '{"d1": "foo"}', '$'
+    COLUMNS (js1 oid[] path '$.d2' DEFAULT '{1}'::int[]::oid[] ON ERROR));
+
 -- JSON_TABLE: Test backward parsing
 
 CREATE VIEW jsonb_table_view2 AS
diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql
index ec330d3b73..71e548e6fa 100644
--- a/src/test/regress/sql/sqljson_queryfuncs.sql
+++ b/src/test/regress/sql/sqljson_queryfuncs.sql
@@ -411,6 +411,33 @@ SELECT JSON_QUERY(js, '$'  RETURNING int DEFAULT (SELECT 1) ON ERROR) FROM test_
 DROP TABLE test_jsonb_mutability;
 DROP FUNCTION ret_setint;
 
+CREATE DOMAIN jsonb_test_domain AS text CHECK (value <> 'foo');
+SELECT JSON_VALUE(jsonb '{"d1": "H"}', '$.a2' RETURNING jsonb_test_domain DEFAULT 'foo'::jsonb_test_domain ON ERROR);
+SELECT JSON_VALUE(jsonb '{"d1": "H"}', '$.a2' RETURNING jsonb_test_domain DEFAULT 'foo1'::jsonb_test_domain ON ERROR);
+SELECT JSON_VALUE(jsonb '{"d1": "H"}', '$.a2' RETURNING jsonb_test_domain DEFAULT '"foo1"'::jsonb::text ON ERROR);
+SELECT JSON_VALUE(jsonb '{"d1": "foo"}', '$.a2' RETURNING jsonb_test_domain DEFAULT 'foo1'::jsonb_test_domain ON ERROR);
+
+-- Check the cases where a coercion-related expression is masking an
+-- unsupported expressions
+
+-- CoerceViaIO
+SELECT JSON_QUERY('"a"', '$.a'  RETURNING int DEFAULT (SELECT '"1"')::jsonb ON ERROR);
+-- CoerceToDomain
+SELECT JSON_QUERY('"a"', '$.a' RETURNING jsonb_test_domain DEFAULT (select '"1"')::jsonb_test_domain ON ERROR);
+-- RelabelType
+SELECT JSON_QUERY('"a"', '$.a'  RETURNING int DEFAULT (SELECT 1)::oid::int ON ERROR);
+-- ArrayCoerceExpr
+SELECT JSON_QUERY('"a"', '$.a'  RETURNING int[] DEFAULT (SELECT '{1}')::oid[]::int[] ON ERROR);
+-- CollateExpr
+SELECT JSON_QUERY('"a"', '$.a'  RETURNING int[] DEFAULT (SELECT '{1}')::text COLLATE "C" ON ERROR);
+-- ConvertRowtypeExpr
+CREATE TABLE someparent (a int);
+CREATE TABLE somechild () INHERITS (someparent);
+SELECT JSON_QUERY('"a"', '$.a'  RETURNING someparent DEFAULT (SELECT '(1)')::somechild::someparent ON ERROR);
+
+DROP DOMAIN jsonb_test_domain;
+DROP TABLE someparent, somechild;
+
 -- Extension: non-constant JSON path
 SELECT JSON_EXISTS(jsonb '{"a": 123}', '$' || '.' || 'a');
 SELECT JSON_VALUE(jsonb '{"a": 123}', '$' || '.' || 'a');
-- 
2.43.0

