On 2021/11/20 1:16, Bharath Rupireddy wrote:
With the existing code, it emits "" for message_primary[0] == '\0'
cases but with the patch it emits "could not obtain message string for
remote error".

Yes.


Well, in that case, why can't we get rid of "(message_primary != NULL"
and just have "message_primary[0] != '\0' ? errmsg_internal("%s",
message_primary) : errmsg("could not obtain message string for remote
error")" ?

That's possible if we can confirm that PQerrorMessage() never returns
NULL all the cases. I'm not sure how much it's worth doing that, though..
It seems more robust to check also NULL there.


BTW, we might have to fix it in dblink_res_error too?

Yeah, that's good idea. I included that change in the patch. Attached.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index c8b0b4ff3f..d73c616f4f 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -2801,7 +2801,8 @@ dblink_res_error(PGconn *conn, const char *conname, 
PGresult *res,
 
        ereport(level,
                        (errcode(sqlstate),
-                        message_primary ? errmsg_internal("%s", 
message_primary) :
+                        (message_primary != NULL && message_primary[0] != 
'\0') ?
+                        errmsg_internal("%s", message_primary) :
                         errmsg("could not obtain message string for remote 
error"),
                         message_detail ? errdetail_internal("%s", 
message_detail) : 0,
                         message_hint ? errhint("%s", message_hint) : 0,
diff --git a/contrib/postgres_fdw/connection.c 
b/contrib/postgres_fdw/connection.c
index 4aff315b7c..5c0137220a 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -824,7 +824,8 @@ pgfdw_report_error(int elevel, PGresult *res, PGconn *conn,
 
                ereport(elevel,
                                (errcode(sqlstate),
-                                message_primary ? errmsg_internal("%s", 
message_primary) :
+                                (message_primary != NULL && message_primary[0] 
!= '\0') ?
+                                errmsg_internal("%s", message_primary) :
                                 errmsg("could not obtain message string for 
remote error"),
                                 message_detail ? errdetail_internal("%s", 
message_detail) : 0,
                                 message_hint ? errhint("%s", message_hint) : 0,

Reply via email to