pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: issue in pgfdw_report_error()?
Date: Mon, 22 Nov 2021 11:47:03 +0900
Message-ID: <192a2107-7e37-64d6-c5b3-c45448ba2e9e@oss.nttdata.com> (raw )
In-Reply-To: <CALj2ACVwi4jSh-4A-Kny8M6bOsypLpi8dNycEtVg4S2qsBDAfQ@mail.gmail.com >
References: <477c16c8-7ea4-20fc-38d5-ed3a77ed616c@oss.nttdata.com >
<CALj2ACVt7HcH+qD7e-grw34eyfV6+WwZYE82--rwOik==1wWBg@mail.gmail.com >
<41aae5d4-4d11-85f6-8f32-1bb5349ce959@oss.nttdata.com >
<CALj2ACVwi4jSh-4A-Kny8M6bOsypLpi8dNycEtVg4S2qsBDAfQ@mail.gmail.com >
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,
Attachments:
[text/plain] pgfdw_report_error_v2.patch (1.3K, ../192a2107-7e37-64d6-c5b3-c45448ba2e9e@oss.nttdata.com/2-pgfdw_report_error_v2.patch)
download | inline diff:
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,
view thread (7+ messages) latest in thread
Message-ID: <192a2107-7e37-64d6-c5b3-c45448ba2e9e@oss.nttdata.com>
Permalink: ../192a2107-7e37-64d6-c5b3-c45448ba2e9e@oss.nttdata.com/
Also on: postgresql.org/message-id/192a2107-7e37-64d6-c5b3-c45448ba2e9e@oss.nttdata.com
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, bharath.rupireddyforpostgres@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: issue in pgfdw_report_error()?
In-Reply-To: <192a2107-7e37-64d6-c5b3-c45448ba2e9e@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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