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 
Suggested-by: Jian He 
Discussion: https://postgr.es/m/CACJufxG_e0QLCgaELrr2ZNz7AxPeGCNKAORe3fHtFCQLsH4J4Q@mail.gmail.com

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 41af28cb1ebe358adb6428beb7beb64e94408535..852186312c59e7b59f48d8ae44d35e7184b55785 100644 (file)
--- 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 4c98d7a046c81fc3c2ef382ecad6978017f91ab2..34ac17868b5e9b1d5faf7f121792d07fca5e2237 100644 (file)
--- 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 99d3101f6b2b36e6c1e2ccddd00fe5ac219dea4b..37f2cba0ef05cd54e5f0dafdadb3673ed4e4b045 100644 (file)
--- 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 103572ed932cc098f0b2c485281790982fd2a923..e74dc1b2d429733109fc5ff0fc7f68951aeddf68 100644 (file)
--- 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 0cab89084375662249ed71164a1d9e69721207a9..c5f34efe271e607b4887d6471e5a60f4e9a36b82 100644 (file)
--- 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 6b0172f4275925c15dbc15bb32ed615091d8df27..0052c1f0ee266bd9b641ceff90724b2787e55772 100644 (file)
--- 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 4d3964488dd5d5631f1786d9f62a9744624cafdc..0bcc1ac569d9f65eb68bd2c3ec47c14a512eed56 100644 (file)
--- 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 a00eec8a6f4029da60a73c015b17ece94aba6e33..9eecd97f45c49641fe93daac2b9d9f9f7598c4eb 100644 (file)
--- 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 9e86b0da10c5f6d094bfc081db026e00f0686b0d..49b014b1ec856475d60759ee47df9ad0186d2eca 100644 (file)
--- 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 3752ccc446b0472f3d046b51f96543a67a975c3a..29c0c6ba529baebaa133a59c20f3c1f7a5053578 100644 (file)
--- 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