Re: issue in pgfdw_report_error()?

On Fri, Nov 19, 2021 at 1:48 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
> Hi,
>
> pgfdw_report_error() in postgres_fdw is implemented to report the message
> "could not obtain ..." if message_primary is NULL as follows.
> But, just before this ereport(), message_primary is set to
> pchomp(PQerrorMessage()) if it's NULL. So ISTM that message_primary is
> always not NULL in ereport() and the message "could not obtain ..." is
> never reported. Is this a bug?
>
> -------------------
> if (message_primary == NULL)
>         message_primary = pchomp(PQerrorMessage(conn));
>
> ereport(elevel,
>                 (errcode(sqlstate),
>                  message_primary ? errmsg_internal("%s", message_primary) :
>                  errmsg("could not obtain message string for remote error"),
> -------------------
>
>
> 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"),
> -------------------

What if conn->errorMessage.data is NULL and PQerrorMessage returns it?
The message_primary can still be NULL right?

I see the other places where PQerrorMessage is used, they do check for
the NULL value in some places the others don't do.

in dblink.c:
    msg = PQerrorMessage(conn);
    if (msg == NULL || msg[0] == '\0')
        PG_RETURN_TEXT_P(cstring_to_text("OK"));

Regards,
Bharath Rupireddy.