Some doubious error messages and comments - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Some doubious error messages and comments
Date
Msg-id 20210428.173633.1525059946206239295.horikyota.ntt@gmail.com
Whole thread Raw
Responses Re: Some doubious error messages and comments  (Justin Pryzby <pryzby@telsasoft.com>)
List pgsql-hackers
Hello.

0001: I found some typos in a error message and a comment.

multirangetypes.c: 1420
> errmsg("range_intersect_agg must be called with a multirange")));

This "range_intersect_agg" looks like a typo of "multirange_..".

operatorcmds.c:303
> * Look up a join estimator function ny name, and verify that it has the

"ny" looks like a typo of "by".



0002: The following messages are substantially same and are uselessly
split into separate messages. I'm not sure any compiler complains
about using %zu for int, explicit casting would work in that case.

be-secure-gssapi.c:351
>    (errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)",
>            (size_t) input.length,
>            PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32))));
be-secure-gssapi.c:570
>    (errmsg("oversize GSSAPI packet sent by the client (%zu > %d)",
>            (size_t) input.length,
>            PQ_GSS_RECV_BUFFER_SIZE)));



0003: The messages below seems to be a bit unclear.  I'm not sure they
worth doing.

conversioncmds.c: 130
 errmsg("encoding conversion function %s returned incorrect result for empty input",

This is not wrong at all, but another message just above is saying
that "encoding conversion function %s must return type %s".  Why
aren't we explicit here, like this?

"encoding conversion function %s must return zero for empty input"


typecmds.c:4294
>    if (requireSuper)
>        if (!superuser())
>            ereport(ERROR,
>                     errmsg("must be superuser to alter a type")));

Where, requireSuper varies depending on the set of operations but the
description looks like describing general behavior. I'm not sure but
something like the following might be better?

+         errmsg("must be superuser to perform all operations")));
+         errmsg("some of the operations require superuser privilege")));

Any opinions or suggestions?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 1634d8dd87b1474f60cfd6689c1e58acc87c1cfc Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Wed, 28 Apr 2021 17:23:52 +0900
Subject: [PATCH 1/3] Fix typos

---
 src/backend/commands/operatorcmds.c     | 2 +-
 src/backend/utils/adt/multirangetypes.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c
index 809043c5d1..ba456c4dd1 100644
--- a/src/backend/commands/operatorcmds.c
+++ b/src/backend/commands/operatorcmds.c
@@ -300,7 +300,7 @@ ValidateRestrictionEstimator(List *restrictionName)
 }
 
 /*
- * Look up a join estimator function ny name, and verify that it has the
+ * Look up a join estimator function by name, and verify that it has the
  * correct signature and we have the permissions to attach it to an
  * operator.
  */
diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c
index 0b81649779..2583ddeedf 100644
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -1417,7 +1417,7 @@ multirange_intersect_agg_transfn(PG_FUNCTION_ARGS)
     if (!type_is_multirange(mltrngtypoid))
         ereport(ERROR,
                 (errcode(ERRCODE_DATATYPE_MISMATCH),
-                 errmsg("range_intersect_agg must be called with a multirange")));
+                 errmsg("multirange_intersect_agg must be called with a multirange")));
 
     typcache = multirange_get_typcache(fcinfo, mltrngtypoid);
 
-- 
2.27.0

From 14a6b4b81d3b4d35bf690e1c240975bd888531f3 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Wed, 28 Apr 2021 17:24:31 +0900
Subject: [PATCH 2/3] Consolidate substantially same messages

---
 src/backend/libpq/be-secure-gssapi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
index 316ca65db5..a335f78d47 100644
--- a/src/backend/libpq/be-secure-gssapi.c
+++ b/src/backend/libpq/be-secure-gssapi.c
@@ -567,9 +567,9 @@ secure_open_gssapi(Port *port)
         if (input.length > PQ_GSS_RECV_BUFFER_SIZE)
         {
             ereport(COMMERROR,
-                    (errmsg("oversize GSSAPI packet sent by the client (%zu > %d)",
+                    (errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)",
                             (size_t) input.length,
-                            PQ_GSS_RECV_BUFFER_SIZE)));
+                            (size_t) PQ_GSS_RECV_BUFFER_SIZE)));
             return -1;
         }
 
-- 
2.27.0

From 0db041334e0fc4df46f1d98e75e984b04777befe Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Wed, 28 Apr 2021 17:25:36 +0900
Subject: [PATCH 3/3] Clarify merror messages

---
 src/backend/commands/conversioncmds.c | 2 +-
 src/backend/commands/typecmds.c       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/conversioncmds.c b/src/backend/commands/conversioncmds.c
index 5fed97a2f9..307afb909d 100644
--- a/src/backend/commands/conversioncmds.c
+++ b/src/backend/commands/conversioncmds.c
@@ -127,7 +127,7 @@ CreateConversionCommand(CreateConversionStmt *stmt)
     if (DatumGetInt32(funcresult) != 0)
         ereport(ERROR,
                 (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                 errmsg("encoding conversion function %s returned incorrect result for empty input",
+                 errmsg("encoding conversion function %s must return zero for empty input",
                         NameListToString(func_name))));
 
     /*
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 036fa69d17..3dd4323386 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -4291,7 +4291,7 @@ AlterType(AlterTypeStmt *stmt)
         if (!superuser())
             ereport(ERROR,
                     (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-                     errmsg("must be superuser to alter a type")));
+                     errmsg("must be superuser to perform all operations")));
     }
     else
     {
-- 
2.27.0


pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: Result Cache node shows per-worker info even for workers not launched
Next
From: Aleksander Alekseev
Date:
Subject: Re: Better sanity checking for message length words