pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedSQLJSON: errmsg(" .. should ...") -> must
3+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2025-01-21 db19a5061c dbc3499a0b Reword recent error messages: "should" -> "must"
* SQLJSON: errmsg(" .. should ...") -> must
@ 2025-01-13 18:19 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 1 reply; 3+ messages in thread
From: Alvaro Herrera @ 2025-01-13 18:19 UTC (permalink / raw)
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Amit Langote <amitlangote09@gmail.com>
Hi
There's a few recent SQL/JSON error messages in which we say something
"should" be something else. We avoid this, so I think we shouldn't use
it here either. (There's also wparser_def.c which uses "should" in that
way, and I think it's there just because it's a dark unvisited corner
that's been wrong for awhile.)
While at it, I wasn't sure about the following:
errmsg("JSON path expression in JSON_QUERY must return single item without wrapper")
It seems a strange phrasing: I read it as meaning that the code is
expecting that the path expression returns a single item and no wrapper
around it. But what it really means is that if the user specifies that
no wrapping is required, then a single item must be returned (so the
"without wrapper" applies to the JSON_QUERY, not to the single item).
I find this confusing, so I'd change it like this:
errmsg("JSON path expression in JSON_QUERY must return single item when no wrapper is requested")
which I think is clearer.
Patch attached, for branch master. This applies to 17 cleanly, so I
lean towards backpatching it all there. (The wparser_def.c changes go
much longer back, but I see little point in backpatching those any
further.)
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Postgres is bloatware by design: it was built to house
PhD theses." (Joey Hellerstein, SIGMOD annual conference 2002)
Attachments:
0001-error-messages-must-use-must-rather-than-should.patch (text/x-diff, 6.8K) download
From ae8f41ec3955055832e250bcd1419a2adff4e582 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@alvh.no-ip.org>
Date: Mon, 13 Jan 2025 19:10:35 +0100
Subject: [PATCH] error messages must use 'must' rather than 'should'
---
src/backend/tsearch/wparser_def.c | 8 ++++----
src/backend/utils/adt/jsonpath_exec.c | 12 ++++++------
src/test/regress/expected/sqljson_jsontable.out | 2 +-
src/test/regress/expected/sqljson_queryfuncs.out | 8 ++++----
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/src/backend/tsearch/wparser_def.c b/src/backend/tsearch/wparser_def.c
index f26923d044b..79bcd32a063 100644
--- a/src/backend/tsearch/wparser_def.c
+++ b/src/backend/tsearch/wparser_def.c
@@ -2671,19 +2671,19 @@ prsd_headline(PG_FUNCTION_ARGS)
if (min_words >= max_words)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("MinWords should be less than MaxWords")));
+ errmsg("%s must be less than %s", "MinWords", "MaxWords")));
if (min_words <= 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("MinWords should be positive")));
+ errmsg("%s must be positive", "MinWords")));
if (shortword < 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("ShortWord should be >= 0")));
+ errmsg("%s must be >= 0", "ShortWord")));
if (max_fragments < 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("MaxFragments should be >= 0")));
+ errmsg("%s must be >= 0", "MaxFragments")));
}
/* Locate words and phrases matching the query */
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index f6dfcb11a62..960fdec3dba 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -3977,13 +3977,13 @@ JsonPathQuery(Datum jb, JsonPath *jp, JsonWrapper wrapper, bool *empty,
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",
+ errmsg("JSON path expression for column \"%s\" must return single item when no wrapper is requested",
column_name),
errhint("Use the WITH WRAPPER clause to wrap SQL/JSON items into an 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"),
+ errmsg("JSON path expression in JSON_QUERY must return single item when no wrapper is requested"),
errhint("Use the WITH WRAPPER clause to wrap SQL/JSON items into an array.")));
}
@@ -4041,12 +4041,12 @@ JsonPathValue(Datum jb, JsonPath *jp, bool *empty, bool *error, List *vars,
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",
+ errmsg("JSON path expression for column \"%s\" must 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")));
+ errmsg("JSON path expression in JSON_VALUE must return single scalar item")));
}
res = JsonValueListHead(&found);
@@ -4065,12 +4065,12 @@ JsonPathValue(Datum jb, JsonPath *jp, bool *empty, bool *error, List *vars,
if (column_name)
ereport(ERROR,
(errcode(ERRCODE_SQL_JSON_SCALAR_REQUIRED),
- errmsg("JSON path expression for column \"%s\" should return single scalar item",
+ errmsg("JSON path expression for column \"%s\" must 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")));
+ errmsg("JSON path expression in JSON_VALUE must return single scalar item")));
}
if (res->type == jbvNull)
diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out
index d62d32241d3..458c5aaa5b0 100644
--- a/src/test/regress/expected/sqljson_jsontable.out
+++ b/src/test/regress/expected/sqljson_jsontable.out
@@ -710,7 +710,7 @@ 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
+ERROR: JSON path expression for column "b" must return single item when no wrapper is requested
HINT: Use the WITH WRAPPER clause to wrap SQL/JSON items into an array.
-- JSON_TABLE: nested paths
-- Duplicate path names
diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index 329ef674961..5a35aeb7bba 100644
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -344,7 +344,7 @@ SELECT JSON_VALUE(jsonb '[]', '$');
(1 row)
SELECT JSON_VALUE(jsonb '[]', '$' ERROR ON ERROR);
-ERROR: JSON path expression in JSON_VALUE should return single scalar item
+ERROR: JSON path expression in JSON_VALUE must return single scalar item
SELECT JSON_VALUE(jsonb '{}', '$');
json_value
------------
@@ -352,7 +352,7 @@ SELECT JSON_VALUE(jsonb '{}', '$');
(1 row)
SELECT JSON_VALUE(jsonb '{}', '$' ERROR ON ERROR);
-ERROR: JSON path expression in JSON_VALUE should return single scalar item
+ERROR: JSON path expression in JSON_VALUE must return single scalar item
SELECT JSON_VALUE(jsonb '1', '$.a');
json_value
------------
@@ -408,7 +408,7 @@ SELECT JSON_VALUE(jsonb '1', 'lax $.a' DEFAULT '2' ON EMPTY DEFAULT '3' ON ERROR
SELECT JSON_VALUE(jsonb '1', 'lax $.a' ERROR ON EMPTY DEFAULT '3' ON ERROR);
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 single scalar item
+ERROR: JSON path expression in JSON_VALUE must return single scalar item
SELECT JSON_VALUE(jsonb '[1,2]', '$[*]' DEFAULT '0' ON ERROR);
json_value
------------
@@ -806,7 +806,7 @@ SELECT JSON_QUERY(jsonb '[]', '$[*]' ERROR ON ERROR); -- NULL ON EMPTY
(1 row)
SELECT JSON_QUERY(jsonb '[1,2]', '$[*]' ERROR ON ERROR);
-ERROR: JSON path expression in JSON_QUERY should return single item without wrapper
+ERROR: JSON path expression in JSON_QUERY must return single item when no wrapper is requested
HINT: Use the WITH WRAPPER clause to wrap SQL/JSON items into an array.
SELECT JSON_QUERY(jsonb '[1,2]', '$[*]' DEFAULT '"empty"' ON ERROR);
json_query
--
2.39.5
* Re: SQLJSON: errmsg(" .. should ...") -> must
@ 2025-01-21 14:42 Alvaro Herrera <alvherre@alvh.no-ip.org>
parent: Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 1 reply; 3+ messages in thread
From: Alvaro Herrera @ 2025-01-21 14:42 UTC (permalink / raw)
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Amit Langote <amitlangote09@gmail.com>
On 2025-Jan-13, Alvaro Herrera wrote:
> There's a few recent SQL/JSON error messages in which we say something
> "should" be something else. We avoid this, so I think we shouldn't use
> it here either.
Pushed.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: SQLJSON: errmsg(" .. should ...") -> must
@ 2025-01-22 06:41 Amit Langote <amitlangote09@gmail.com>
parent: Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 3+ messages in thread
From: Amit Langote @ 2025-01-22 06:41 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, Jan 21, 2025 at 11:42 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> On 2025-Jan-13, Alvaro Herrera wrote:
>
> > There's a few recent SQL/JSON error messages in which we say something
> > "should" be something else. We avoid this, so I think we shouldn't use
> > it here either.
>
> Pushed.
Sorry Alvaro for not noticing sooner.
I looked at db19a5061ce and it all looks fine to me.
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2025-01-22 06:41 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-01-13 18:19 SQLJSON: errmsg(" .. should ...") -> must Alvaro Herrera <alvherre@alvh.no-ip.org>
2025-01-21 14:42 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2025-01-22 06:41 ` Amit Langote <amitlangote09@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox