Horton

Thread

Commits

  1. Fix missed cases in libpq's error handling.

  2. Rearrange libpq's error reporting to avoid duplicated error text.

  1. error handling in pqRowProcessor broken

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-04-19T14:34:36Z

    The error handling for pqRowProcessor is described as

  *    Add the received row to the current async result (conn->result).
  *    Returns 1 if OK, 0 if error occurred.
  *
  * On error, *errmsgp can be set to an error string to be returned.
  * If it is left NULL, the error is presumed to be "out of memory".

I find that this doesn't work anymore.  If you set *errmsgp = "some 
message" and return 0, then psql will just print a result set with zero 
rows.

Bisecting points to

commit 618c16707a6d6e8f5c83ede2092975e4670201ad
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Fri Feb 18 15:35:15 2022 -0500

     Rearrange libpq's error reporting to avoid duplicated error text.

It is very uncommon to get an error from pqRowProcessor().  To 
reproduce, I inserted this code:

diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index c7c48d07dc..9c1b33c6e2 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1124,6 +1124,12 @@ pqRowProcessor(PGconn *conn, const char **errmsgp)
             return 0;
     }

+   if (nfields == 7)
+   {
+       *errmsgp = "gotcha";
+       goto fail;
+   }
+
     /*
      * Basically we just allocate space in the PGresult for each field and
      * copy the data over.

This will produce assorted failures in the regression tests that 
illustrate the effect.

(Even before the above commit, the handling of the returned message was 
a bit weird:  The error output was just the message string, without any 
prefix like "ERROR:".)

  2. Re: error handling in pqRowProcessor broken

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-04-19T14:54:30Z

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> The error handling for pqRowProcessor is described as
>   *    Add the received row to the current async result (conn->result).
>   *    Returns 1 if OK, 0 if error occurred.
>   *
>   * On error, *errmsgp can be set to an error string to be returned.
>   * If it is left NULL, the error is presumed to be "out of memory".

> I find that this doesn't work anymore.

Will look into it, thanks for reporting.

(Hmm, seems like this API spec is deficient anyway.  Is the error
string to be freed later?  Is it already translated?)

			regards, tom lane

  3. Re: error handling in pqRowProcessor broken

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-04-19T19:16:05Z

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> I find that this doesn't work anymore.  If you set *errmsgp = "some
> message" and return 0, then psql will just print a result set with zero
> rows.

Ah, I see the problem: a few places in fe-protocol3 didn't get the memo
that conn->error_result represents a "pending" PGresult that hasn't
been constructed yet.  The attached fixes it for me --- can you try it
on whatever test case led you to this?

> (Even before the above commit, the handling of the returned message was
> a bit weird: The error output was just the message string, without any
> prefix like "ERROR:".)

libpq's always acted that way for internally-generated messages.
Most of them are so rare that we're probably not used to seeing 'em.
Perhaps there's a case for making it more verbose, but right now
doesn't seem like the time to undertake that.

			regards, tom lane

  4. Re: error handling in pqRowProcessor broken

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-04-21T20:18:52Z

    On 19.04.22 21:16, Tom Lane wrote:
> Peter Eisentraut<peter.eisentraut@enterprisedb.com>  writes:
>> I find that this doesn't work anymore.  If you set *errmsgp = "some
>> message" and return 0, then psql will just print a result set with zero
>> rows.
> Ah, I see the problem: a few places in fe-protocol3 didn't get the memo
> that conn->error_result represents a "pending" PGresult that hasn't
> been constructed yet.  The attached fixes it for me --- can you try it
> on whatever test case led you to this?

Your patch fixes it for me.

  5. Re: error handling in pqRowProcessor broken

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-04-21T21:13:50Z

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 19.04.22 21:16, Tom Lane wrote:
>> Ah, I see the problem: a few places in fe-protocol3 didn't get the memo
>> that conn->error_result represents a "pending" PGresult that hasn't
>> been constructed yet.  The attached fixes it for me --- can you try it
>> on whatever test case led you to this?

> Your patch fixes it for me.

Thanks for testing.  I pushed a cosmetically-polished version of that.

			regards, tom lane