Fix missed cases in libpq's error handling.

Commit 618c16707 invented an "error_result" flag in PGconn, which
intends to represent the state that we have an error condition and
need to build a PGRES_FATAL_ERROR PGresult from the message text in
conn->errorMessage, but have not yet done so.  (Postponing construction
of the error object simplifies dealing with out-of-memory conditions
and with concatenation of messages for multiple errors.)  For nearly all
purposes, this "virtual" PGresult object should act the same as if it
were already materialized.  But a couple of places in fe-protocol3.c
didn't get that memo, and were only testing conn->result as they used
to, without also checking conn->error_result.

In hopes of reducing the probability of similar mistakes in future,
I invented a pgHavePendingResult() macro that includes both tests.

Per report from Peter Eisentraut.

Discussion: https://postgr.es/m/b52277b9-fa66-b027-4a37-fb8989c73ff8@enterprisedb.com

diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 78cff4475cc330eda713198df9d9f05a4c32eb17..919cf5741d40dbaf05b054c9ab89836f48257b15 100644 (file)
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1196,6 +1196,7 @@ pqSaveParameterStatus(PGconn *conn, const char *name, const char *value)
  *   Returns 1 if OK, 0 if error occurred.
  *
  * On error, *errmsgp can be set to an error string to be returned.
+ * (Such a string should already be translated via libpq_gettext().)
  * If it is left NULL, the error is presumed to be "out of memory".
  *
  * In single-row mode, we create a new result holding just the current row,
@@ -1986,7 +1987,7 @@ PQsetSingleRowMode(PGconn *conn)
        (conn->cmd_queue_head->queryclass != PGQUERY_SIMPLE &&
         conn->cmd_queue_head->queryclass != PGQUERY_EXTENDED))
        return 0;
-   if (conn->result || conn->error_result)
+   if (pgHavePendingResult(conn))
        return 0;
 
    /* OK, set flag */
@@ -2941,7 +2942,7 @@ PQfn(PGconn *conn,
    }
 
    if (conn->sock == PGINVALID_SOCKET || conn->asyncStatus != PGASYNC_IDLE ||
-       conn->result || conn->error_result)
+       pgHavePendingResult(conn))
    {
        appendPQExpBufferStr(&conn->errorMessage,
                             libpq_gettext("connection in wrong state\n"));

diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 94b4a448b9070740d812eaaf9a3887a8120ba8d6..10c76daf6ed594639abc0b47cd2c153aa980bb00 100644 (file)
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -209,7 +209,7 @@ pqParseInput3(PGconn *conn)
                case 'C':       /* command complete */
                    if (pqGets(&conn->workBuffer, conn))
                        return;
-                   if (conn->result == NULL)
+                   if (!pgHavePendingResult(conn))
                    {
                        conn->result = PQmakeEmptyPGresult(conn,
                                                           PGRES_COMMAND_OK);
@@ -263,7 +263,7 @@ pqParseInput3(PGconn *conn)
                    }
                    break;
                case 'I':       /* empty query */
-                   if (conn->result == NULL)
+                   if (!pgHavePendingResult(conn))
                    {
                        conn->result = PQmakeEmptyPGresult(conn,
                                                           PGRES_EMPTY_QUERY);
@@ -281,7 +281,7 @@ pqParseInput3(PGconn *conn)
                    if (conn->cmd_queue_head &&
                        conn->cmd_queue_head->queryclass == PGQUERY_PREPARE)
                    {
-                       if (conn->result == NULL)
+                       if (!pgHavePendingResult(conn))
                        {
                            conn->result = PQmakeEmptyPGresult(conn,
                                                               PGRES_COMMAND_OK);
@@ -362,7 +362,7 @@ pqParseInput3(PGconn *conn)
                    if (conn->cmd_queue_head &&
                        conn->cmd_queue_head->queryclass == PGQUERY_DESCRIBE)
                    {
-                       if (conn->result == NULL)
+                       if (!pgHavePendingResult(conn))
                        {
                            conn->result = PQmakeEmptyPGresult(conn,
                                                               PGRES_COMMAND_OK);
@@ -2133,7 +2133,7 @@ pqFunctionCall3(PGconn *conn, Oid fnid,
                 * report COMMAND_OK.  Otherwise, the backend violated the
                 * protocol, so complain.
                 */
-               if (!(conn->result || conn->error_result))
+               if (!pgHavePendingResult(conn))
                {
                    if (status == PGRES_COMMAND_OK)
                    {

diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index e0cee4b142f2a6ff43a3ec62c1aa1ad46495be54..3db6a17db4da1500d2a034671b29a87a188aa373 100644 (file)
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -852,6 +852,14 @@ extern void pqTraceOutputNoTypeByteMessage(PGconn *conn, const char *message);
    (resetPQExpBuffer(&(conn)->errorMessage), \
     (conn)->errorReported = 0)
 
+/*
+ * Check whether we have a PGresult pending to be returned --- either a
+ * constructed one in conn->result, or a "virtual" error result that we
+ * don't intend to materialize until the end of the query cycle.
+ */
+#define pgHavePendingResult(conn) \
+   ((conn)->result != NULL || (conn)->error_result)
+
 /*
  * this is so that we can check if a connection is non-blocking internally
  * without the overhead of a function call