On Fri, Nov 19, 2021 at 8:48 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
> On 2021/11/19 21:57, Bharath Rupireddy wrote:
> >> If this is a bug, IMO the following change needs to be applied. Thought?
> >>
> >> -------------------
> >> 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"),
> >> -------------------
>
> I attached the patch.
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".
- message_primary ? errmsg_internal("%s", message_primary) :
+ (message_primary != NULL && message_primary[0] != '\0') ?
+ errmsg_internal("%s", message_primary) :
>
> > What if conn->errorMessage.data is NULL and PQerrorMessage returns it?
> > The message_primary can still be NULL right?
>
> Since conn->errorMessage is initialized by initPQExpBuffer(),
> PQerrorMessage() seems not to return NULL. But *if* it returns NULL,
> pchomp(NULL) is executed and would cause a segmentation fault.
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")" ?
BTW, we might have to fix it in dblink_res_error too?
/*
* If we don't get a message from the PGresult, try the PGconn. This is
* needed because for connection-level failures, PQexec may just return
* NULL, not a PGresult at all.
*/
if (message_primary == NULL)
message_primary = pchomp(PQerrorMessage(conn));
ereport(level,
(errcode(sqlstate),
message_primary ? 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,
Regards,
Bharath Rupireddy.