From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: issue in pgfdw_report_error()? |
Date: | 2021-11-19 16:16:50 |
Message-ID: | CALj2ACVwi4jSh-4A-Kny8M6bOsypLpi8dNycEtVg4S2qsBDAfQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Nov 19, 2021 at 8:48 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)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.
From | Date | Subject | |
---|---|---|---|
Next Message | Bharath Rupireddy | 2021-11-19 16:23:21 | Re: A spot of redundant initialization of brin memtuple |
Previous Message | Mark Dilger | 2021-11-19 16:12:27 | Re: Non-superuser subscription owners |