SQL/JSON: Improve some error messages

This improves some error messages emitted by SQL/JSON query functions
by mentioning column name when available, such as when they are
invoked as part of evaluating JSON_TABLE() columns.  To do so, a new
field column_name is added to both JsonFuncExpr and JsonExpr that is
only populated when creating those nodes for transformed JSON_TABLE()
columns.

While at it, relevant error messages are reworded for clarity.

Reported-by: Jian He <jian.universality@gmail.com>
Suggested-by: Jian He <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxG_e0QLCgaELrr2ZNz7AxPeGCNKAORe3fHtFCQLsH4J4Q@mail.gmail.com

10 files changed, 96 insertions, 57 deletions

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 41af28cb1eb..852186312c5 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -4312,7 +4312,8 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
 		case JSON_QUERY_OP:
 			*op->resvalue = JsonPathQuery(item, path, jsexpr->wrapper, &empty,
 										  !throw_error ? &error : NULL,
-										  jsestate->args);
+										  jsestate->args,
+										  jsexpr->column_name);
 
 			*op->resnull = (DatumGetPointer(*op->resvalue) == NULL);
 
@@ -4337,7 +4338,8 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
 			{
 				JsonbValue *jbv = JsonPathValue(item, path, &empty,
 												!throw_error ? &error : NULL,
-												jsestate->args);
+												jsestate->args,
+												jsexpr->column_name);
 
 				if (jbv == NULL)
 				{
@@ -4407,30 +4409,33 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
 	/* Handle ON EMPTY. */
 	if (empty)
 	{
+		*op->resvalue = (Datum) 0;
+		*op->resnull = true;
 		if (jsexpr->on_empty)
 		{
-			if (jsexpr->on_empty->btype == JSON_BEHAVIOR_ERROR)
-				ereport(ERROR,
-						errcode(ERRCODE_NO_SQL_JSON_ITEM),
-						errmsg("no SQL/JSON item"));
-			else
+			if (jsexpr->on_empty->btype != JSON_BEHAVIOR_ERROR)
+			{
 				jsestate->empty.value = BoolGetDatum(true);
-
-			Assert(jsestate->jump_empty >= 0);
-			return jsestate->jump_empty;
+				Assert(jsestate->jump_empty >= 0);
+				return jsestate->jump_empty;
+			}
+		}
+		else if (jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR)
+		{
+			jsestate->error.value = BoolGetDatum(true);
+			Assert(!throw_error && jsestate->jump_error >= 0);
+			return jsestate->jump_error;
 		}
-		else if (jsexpr->on_error->btype == JSON_BEHAVIOR_ERROR)
+
+		if (jsexpr->column_name)
 			ereport(ERROR,
 					errcode(ERRCODE_NO_SQL_JSON_ITEM),
-					errmsg("no SQL/JSON item"));
+					errmsg("no SQL/JSON item found for specified path of column \"%s\"",
+						   jsexpr->column_name));
 		else
-			jsestate->error.value = BoolGetDatum(true);
-
-		*op->resvalue = (Datum) 0;
-		*op->resnull = true;
-
-		Assert(!throw_error && jsestate->jump_error >= 0);
-		return jsestate->jump_error;
+			ereport(ERROR,
+					errcode(ERRCODE_NO_SQL_JSON_ITEM),
+					errmsg("no SQL/JSON item found for specified path"));
 	}
 
 	/*
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 4c98d7a046c..34ac17868b5 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -4311,6 +4311,7 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func)
 	jsexpr = makeNode(JsonExpr);
 	jsexpr->location = func->location;
 	jsexpr->op = func->op;
+	jsexpr->column_name = func->column_name;
 
 	/*
 	 * jsonpath machinery can only handle jsonb documents, so coerce the input
diff --git a/src/backend/parser/parse_jsontable.c b/src/backend/parser/parse_jsontable.c
index 99d3101f6b2..37f2cba0ef0 100644
--- a/src/backend/parser/parse_jsontable.c
+++ b/src/backend/parser/parse_jsontable.c
@@ -402,12 +402,6 @@ transformJsonTableColumn(JsonTableColumn *jtc, Node *contextItemExpr,
 	Node	   *pathspec;
 	JsonFuncExpr *jfexpr = makeNode(JsonFuncExpr);
 
-	/*
-	 * XXX consider inventing JSON_TABLE_VALUE_OP, etc. and pass the column
-	 * name via JsonExpr so that JsonPathValue(), etc. can provide error
-	 * message tailored to JSON_TABLE(), such as by mentioning the column
-	 * names in the message.
-	 */
 	if (jtc->coltype == JTC_REGULAR)
 		jfexpr->op = JSON_VALUE_OP;
 	else if (jtc->coltype == JTC_EXISTS)
@@ -415,6 +409,10 @@ transformJsonTableColumn(JsonTableColumn *jtc, Node *contextItemExpr,
 	else
 		jfexpr->op = JSON_QUERY_OP;
 
+	/* Pass the column name so any runtime JsonExpr errors can print it. */
+	Assert(jtc->name != NULL);
+	jfexpr->column_name = pstrdup(jtc->name);
+
 	jfexpr->context_item = makeJsonValueExpr((Expr *) contextItemExpr, NULL,
 											 makeJsonFormat(JS_FORMAT_DEFAULT,
 															JS_ENC_DEFAULT,
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index 103572ed932..e74dc1b2d42 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -3899,7 +3899,8 @@ JsonPathExists(Datum jb, JsonPath *jp, bool *error, List *vars)
  */
 Datum
 JsonPathQuery(Datum jb, JsonPath *jp, JsonWrapper wrapper, bool *empty,
-			  bool *error, List *vars)
+			  bool *error, List *vars,
+			  const char *column_name)
 {
 	JsonbValue *singleton;
 	bool		wrap;
@@ -3950,10 +3951,17 @@ JsonPathQuery(Datum jb, JsonPath *jp, JsonWrapper wrapper, bool *empty,
 			return (Datum) 0;
 		}
 
-		ereport(ERROR,
-				(errcode(ERRCODE_MORE_THAN_ONE_SQL_JSON_ITEM),
-				 errmsg("JSON path expression in JSON_QUERY should return singleton item without wrapper"),
-				 errhint("Use WITH WRAPPER clause to wrap SQL/JSON item sequence into array.")));
+		if (column_name)
+			ereport(ERROR,
+					(errcode(ERRCODE_MORE_THAN_ONE_SQL_JSON_ITEM),
+					 errmsg("JSON path expression for column \"%s\" should return single item without wrapper",
+							column_name),
+					 errhint("Use WITH WRAPPER clause to wrap SQL/JSON items into array.")));
+		else
+			ereport(ERROR,
+					(errcode(ERRCODE_MORE_THAN_ONE_SQL_JSON_ITEM),
+					 errmsg("JSON path expression in JSON_QUERY should return single item without wrapper"),
+					 errhint("Use WITH WRAPPER clause to wrap SQL/JSON items into array.")));
 	}
 
 	if (singleton)
@@ -3970,7 +3978,8 @@ JsonPathQuery(Datum jb, JsonPath *jp, JsonWrapper wrapper, bool *empty,
  * *error to true.  *empty is set to true if no match is found.
  */
 JsonbValue *
-JsonPathValue(Datum jb, JsonPath *jp, bool *empty, bool *error, List *vars)
+JsonPathValue(Datum jb, JsonPath *jp, bool *empty, bool *error, List *vars,
+			  const char *column_name)
 {
 	JsonbValue *res;
 	JsonValueList found = {0};
@@ -4006,9 +4015,15 @@ JsonPathValue(Datum jb, JsonPath *jp, bool *empty, bool *error, List *vars)
 			return NULL;
 		}
 
-		ereport(ERROR,
-				(errcode(ERRCODE_MORE_THAN_ONE_SQL_JSON_ITEM),
-				 errmsg("JSON path expression in JSON_VALUE should return singleton scalar item")));
+		if (column_name)
+			ereport(ERROR,
+					(errcode(ERRCODE_MORE_THAN_ONE_SQL_JSON_ITEM),
+					 errmsg("JSON path expression for column \"%s\" should return single scalar item",
+							column_name)));
+		else
+			ereport(ERROR,
+					(errcode(ERRCODE_MORE_THAN_ONE_SQL_JSON_ITEM),
+					 errmsg("JSON path expression in JSON_VALUE should return single scalar item")));
 	}
 
 	res = JsonValueListHead(&found);
@@ -4024,9 +4039,15 @@ JsonPathValue(Datum jb, JsonPath *jp, bool *empty, bool *error, List *vars)
 			return NULL;
 		}
 
-		ereport(ERROR,
-				(errcode(ERRCODE_SQL_JSON_SCALAR_REQUIRED),
-				 errmsg("JSON path expression in JSON_VALUE should return singleton scalar item")));
+		if (column_name)
+			ereport(ERROR,
+					(errcode(ERRCODE_SQL_JSON_SCALAR_REQUIRED),
+					 errmsg("JSON path expression for column \"%s\" should return single scalar item",
+							column_name)));
+		else
+			ereport(ERROR,
+					(errcode(ERRCODE_SQL_JSON_SCALAR_REQUIRED),
+					 errmsg("JSON path expression in JSON_VALUE should return single scalar item")));
 	}
 
 	if (res->type == jbvNull)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 0cab8908437..c5f34efe271 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1791,6 +1791,8 @@ typedef struct JsonFuncExpr
 {
 	NodeTag		type;
 	JsonExprOp	op;				/* expression type */
+	char	   *column_name;	/* JSON_TABLE() column name or NULL if this is
+								 * not for a JSON_TABLE() */
 	JsonValueExpr *context_item;	/* context item expression */
 	Node	   *pathspec;		/* JSON path specification expression */
 	List	   *passing;		/* list of PASSING clause arguments, if any */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 6b0172f4275..0052c1f0ee2 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1782,6 +1782,9 @@ typedef struct JsonExpr
 
 	JsonExprOp	op;
 
+	char	   *column_name;	/* JSON_TABLE() column name or NULL if this is
+								 * not for a JSON_TABLE() */
+
 	/* jsonb-valued expression to query */
 	Node	   *formatted_expr;
 
diff --git a/src/include/utils/jsonpath.h b/src/include/utils/jsonpath.h
index 4d3964488dd..0bcc1ac569d 100644
--- a/src/include/utils/jsonpath.h
+++ b/src/include/utils/jsonpath.h
@@ -300,9 +300,11 @@ typedef struct JsonPathVariable
 /* SQL/JSON item */
 extern bool JsonPathExists(Datum jb, JsonPath *path, bool *error, List *vars);
 extern Datum JsonPathQuery(Datum jb, JsonPath *jp, JsonWrapper wrapper,
-						   bool *empty, bool *error, List *vars);
+						   bool *empty, bool *error, List *vars,
+						   const char *column_name);
 extern JsonbValue *JsonPathValue(Datum jb, JsonPath *jp, bool *empty,
-								 bool *error, List *vars);
+								 bool *error, List *vars,
+								 const char *column_name);
 
 extern PGDLLIMPORT const TableFuncRoutine JsonbTableRoutine;
 
diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out
index a00eec8a6f4..9eecd97f45c 100644
--- a/src/test/regress/expected/sqljson_jsontable.out
+++ b/src/test/regress/expected/sqljson_jsontable.out
@@ -492,11 +492,11 @@ FROM
 		ON true;
 ERROR:  invalid input syntax for type integer: "err"
 SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int PATH '$.a' ERROR ON EMPTY)) jt;
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path of column "a"
 SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int PATH 'strict $.a' ERROR ON ERROR) ERROR ON ERROR) jt;
 ERROR:  jsonpath member accessor can only be applied to an object
 SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int PATH 'lax $.a' ERROR ON EMPTY) ERROR ON ERROR) jt;
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path of column "a"
 SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int PATH '$'   DEFAULT 1 ON EMPTY DEFAULT 2 ON ERROR)) jt;
  a 
 ---
@@ -637,6 +637,10 @@ SELECT * FROM JSON_TABLE(jsonb '{"a": 123}', '$' || '.' || 'a' COLUMNS (foo int)
 ERROR:  only string constants are supported in JSON_TABLE path specification
 LINE 1: SELECT * FROM JSON_TABLE(jsonb '{"a": 123}', '$' || '.' || '...
                                                      ^
+-- JsonPathQuery() error message mentioning column name
+SELECT * FROM JSON_TABLE('{"a": [{"b": "1"}, {"b": "2"}]}', '$' COLUMNS (b json path '$.a[*].b' ERROR ON ERROR));
+ERROR:  JSON path expression for column "b" should return single item without wrapper
+HINT:  Use WITH WRAPPER clause to wrap SQL/JSON items into array.
 -- JSON_TABLE: nested paths
 -- Duplicate path names
 SELECT * FROM JSON_TABLE(
@@ -849,7 +853,7 @@ SELECT sub.* FROM s,
 		xx int path '$.c',
 		NESTED PATH '$.a.za[1]' columns (NESTED PATH '$.z21[*]' COLUMNS (z21 int path '$?(@ >= $"x")' ERROR ON ERROR))
 	)) sub;
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path of column "z21"
 -- Parent columns xx1, xx appear before NESTED ones
 SELECT sub.* FROM s,
 	(VALUES (23)) x(x), generate_series(13, 13) y,
diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index 9e86b0da10c..49b014b1ec8 100644
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -339,7 +339,7 @@ SELECT JSON_VALUE(jsonb '[]', '$');
 (1 row)
 
 SELECT JSON_VALUE(jsonb '[]', '$' ERROR ON ERROR);
-ERROR:  JSON path expression in JSON_VALUE should return singleton scalar item
+ERROR:  JSON path expression in JSON_VALUE should return single scalar item
 SELECT JSON_VALUE(jsonb '{}', '$');
  json_value 
 ------------
@@ -347,7 +347,7 @@ SELECT JSON_VALUE(jsonb '{}', '$');
 (1 row)
 
 SELECT JSON_VALUE(jsonb '{}', '$' ERROR ON ERROR);
-ERROR:  JSON path expression in JSON_VALUE should return singleton scalar item
+ERROR:  JSON path expression in JSON_VALUE should return single scalar item
 SELECT JSON_VALUE(jsonb '1', '$.a');
  json_value 
 ------------
@@ -363,9 +363,9 @@ SELECT JSON_VALUE(jsonb '1', 'strict $.a' DEFAULT 'error' ON ERROR);
 (1 row)
 
 SELECT JSON_VALUE(jsonb '1', 'lax $.a' ERROR ON ERROR);
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path
 SELECT JSON_VALUE(jsonb '1', 'lax $.a' ERROR ON EMPTY ERROR ON ERROR);
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path
 SELECT JSON_VALUE(jsonb '1', 'strict $.a' DEFAULT 2 ON ERROR);
  json_value 
 ------------
@@ -397,9 +397,9 @@ SELECT JSON_VALUE(jsonb '1', 'lax $.a' DEFAULT '2' ON EMPTY DEFAULT '3' ON ERROR
 (1 row)
 
 SELECT JSON_VALUE(jsonb '1', 'lax $.a' ERROR ON EMPTY DEFAULT '3' ON ERROR);
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path
 SELECT JSON_VALUE(jsonb '[1,2]', '$[*]' ERROR ON ERROR);
-ERROR:  JSON path expression in JSON_VALUE should return singleton scalar item
+ERROR:  JSON path expression in JSON_VALUE should return single scalar item
 SELECT JSON_VALUE(jsonb '[1,2]', '$[*]' DEFAULT '0' ON ERROR);
  json_value 
 ------------
@@ -758,7 +758,7 @@ SELECT JSON_QUERY(jsonb '[]', '$[*]' EMPTY OBJECT ON EMPTY);
 (1 row)
 
 SELECT JSON_QUERY(jsonb '[]', '$[*]' ERROR ON EMPTY);
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path
 SELECT JSON_QUERY(jsonb '[]', '$[*]' DEFAULT '"empty"' ON EMPTY);
  json_query 
 ------------
@@ -766,18 +766,18 @@ SELECT JSON_QUERY(jsonb '[]', '$[*]' DEFAULT '"empty"' ON EMPTY);
 (1 row)
 
 SELECT JSON_QUERY(jsonb '[]', '$[*]' ERROR ON EMPTY NULL ON ERROR);
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path
 SELECT JSON_QUERY(jsonb '[]', '$[*]' ERROR ON EMPTY EMPTY ARRAY ON ERROR);
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path
 SELECT JSON_QUERY(jsonb '[]', '$[*]' ERROR ON EMPTY EMPTY OBJECT ON ERROR);
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path
 SELECT JSON_QUERY(jsonb '[]', '$[*]' ERROR ON EMPTY ERROR ON ERROR);
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path
 SELECT JSON_QUERY(jsonb '[]', '$[*]' ERROR ON ERROR);
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path
 SELECT JSON_QUERY(jsonb '[1,2]', '$[*]' ERROR ON ERROR);
-ERROR:  JSON path expression in JSON_QUERY should return singleton item without wrapper
-HINT:  Use WITH WRAPPER clause to wrap SQL/JSON item sequence into array.
+ERROR:  JSON path expression in JSON_QUERY should return single item without wrapper
+HINT:  Use WITH WRAPPER clause to wrap SQL/JSON items into array.
 SELECT JSON_QUERY(jsonb '[1,2]', '$[*]' DEFAULT '"empty"' ON ERROR);
  json_query 
 ------------
@@ -1033,7 +1033,7 @@ SELECT JSON_QUERY(jsonb '{"a": 1}', '$.b' RETURNING sqljsonb_int_not_null);
 (1 row)
 
 SELECT JSON_QUERY(jsonb '{"a": 1}', '$.b' RETURNING sqljsonb_int_not_null ERROR ON ERROR);
-ERROR:  no SQL/JSON item
+ERROR:  no SQL/JSON item found for specified path
 -- Test timestamptz passing and output
 SELECT JSON_QUERY(jsonb 'null', '$ts' PASSING timestamptz '2018-02-21 12:34:56 +10' AS ts);
          json_query          
diff --git a/src/test/regress/sql/sqljson_jsontable.sql b/src/test/regress/sql/sqljson_jsontable.sql
index 3752ccc446b..29c0c6ba529 100644
--- a/src/test/regress/sql/sqljson_jsontable.sql
+++ b/src/test/regress/sql/sqljson_jsontable.sql
@@ -290,6 +290,9 @@ FROM JSON_TABLE(
 -- Should fail (not supported)
 SELECT * FROM JSON_TABLE(jsonb '{"a": 123}', '$' || '.' || 'a' COLUMNS (foo int));
 
+-- JsonPathQuery() error message mentioning column name
+SELECT * FROM JSON_TABLE('{"a": [{"b": "1"}, {"b": "2"}]}', '$' COLUMNS (b json path '$.a[*].b' ERROR ON ERROR));
+
 -- JSON_TABLE: nested paths
 
 -- Duplicate path names