From f6f60653da88543f1d1074f217b356dfd0832c58 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Tue, 9 Apr 2024 20:31:30 +0900 Subject: [PATCH v2] JSON_TABLE(): mention column name in some error messages --- src/backend/executor/execExprInterp.c | 42 +++++++++-------- src/backend/parser/parse_expr.c | 1 + src/backend/parser/parse_jsontable.c | 9 ++-- src/backend/utils/adt/jsonpath_exec.c | 45 ++++++++++++++----- src/include/nodes/parsenodes.h | 2 + src/include/nodes/primnodes.h | 3 ++ src/include/utils/jsonpath.h | 6 ++- .../regress/expected/sqljson_jsontable.out | 10 +++-- src/test/regress/sql/sqljson_jsontable.sql | 3 ++ 9 files changed, 80 insertions(+), 41 deletions(-) diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 41af28cb1e..580c897144 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) { @@ -4409,28 +4411,32 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op, { 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; + } + } + else if (jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR) + { + jsestate->error.value = BoolGetDatum(true); - Assert(jsestate->jump_empty >= 0); - return jsestate->jump_empty; + *op->resvalue = (Datum) 0; + *op->resnull = 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 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")); } /* diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 4c98d7a046..34ac17868b 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 99d3101f6b..70b00a45f0 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,9 @@ transformJsonTableColumn(JsonTableColumn *jtc, Node *contextItemExpr, else jfexpr->op = JSON_QUERY_OP; + /* Pass the column name so any runtime JsonExpr errors can print it. */ + 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 103572ed93..1cc6bfec8b 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 singleton item without wrapper", + column_name), + errhint("Use WITH WRAPPER clause to wrap SQL/JSON item sequence into array."))); + else + 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 (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 singleton scalar item", + column_name))); + else + ereport(ERROR, + (errcode(ERRCODE_SQL_JSON_SCALAR_REQUIRED), + errmsg("JSON path expression in JSON_VALUE should return singleton 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_MORE_THAN_ONE_SQL_JSON_ITEM), + errmsg("JSON path expression for column \"%s\" should return singleton scalar item", + column_name))); + else + ereport(ERROR, + (errcode(ERRCODE_SQL_JSON_SCALAR_REQUIRED), + errmsg("JSON path expression in JSON_VALUE should return singleton scalar item"))); } if (res->type == jbvNull) diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index f763f790b1..c0ff75e643 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 dafe93a4c9..9b662b8dd2 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 4d3964488d..0bcc1ac569 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 a00eec8a6f..6bbc7b46cf 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 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 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 singleton item without wrapper +HINT: Use WITH WRAPPER clause to wrap SQL/JSON item sequence 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 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/sql/sqljson_jsontable.sql b/src/test/regress/sql/sqljson_jsontable.sql index 3752ccc446..29c0c6ba52 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 -- 2.43.0