On Sun, Dec 4, 2016 at 11:00 PM, Mithun Cy <mithun...@enterprisedb.com> wrote: > On Fri, Dec 2, 2016 at 8:54 PM, Robert Haas <robertmh...@gmail.com> wrote: >> Couldn't this just be a variable in PQconnectPoll(), instead of adding >> a new structure member? > > I have fixed same with a local variable in PQconnectPoll, Initally I thought > savedMessage should have same visiblitly as errorMessage so we can restore > the errorMessage even outside PQconnectPoll. But that seems not required. > Attacting the new patch which fixes the same.
I think that you need a restoreErrorMessage call here: /* Skip any remaining addresses for this host. */ conn->addr_cur = NULL; if (conn->whichhost + 1 < conn->nconnhost) { conn->status = CONNECTION_NEEDED; restoreErrorMessage(conn, &savedMessage); goto keep_going; } Updated patch attached with that change, the removal of a useless hunk, and the removal of "inline" from the new functions, which seems like second-guessing of whatever decision the compiler chooses to make. Thoughts? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 3b9b263..24fc12d 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -1762,6 +1762,39 @@ connectDBComplete(PGconn *conn) } } +/* + * This subroutine saves conn->errorMessage, which will be restored back by + * restoreErrorMessage subroutine. + */ +static bool +saveErrorMessage(PGconn *conn, PQExpBuffer savedMessage) +{ + initPQExpBuffer(savedMessage); + if (PQExpBufferBroken(savedMessage)) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("out of memory\n")); + return false; + } + + appendPQExpBufferStr(savedMessage, + conn->errorMessage.data); + resetPQExpBuffer(&conn->errorMessage); + return true; +} + +/* + * Restores saved error messages back to conn->errorMessage. + */ +static void +restoreErrorMessage(PGconn *conn, PQExpBuffer savedMessage) +{ + appendPQExpBufferStr(savedMessage, conn->errorMessage.data); + resetPQExpBuffer(&conn->errorMessage); + appendPQExpBufferStr(&conn->errorMessage, savedMessage->data); + termPQExpBuffer(savedMessage); +} + /* ---------------- * PQconnectPoll * @@ -1795,6 +1828,7 @@ PQconnectPoll(PGconn *conn) PGresult *res; char sebuf[256]; int optval; + PQExpBufferData savedMessage; if (conn == NULL) return PGRES_POLLING_FAILED; @@ -2792,11 +2826,26 @@ keep_going: /* We will come back to here until there is if (conn->target_session_attrs != NULL && strcmp(conn->target_session_attrs, "read-write") == 0) { + /* + * We are yet to make a connection. Save all existing error + * messages until we make a successful connection state. + * This is important because PQsendQuery is going to reset + * conn->errorMessage and we will loose error messages + * related to previous hosts we have tried to connect and + * failed. + */ + if (!saveErrorMessage(conn, &savedMessage)) + goto error_return; + conn->status = CONNECTION_OK; if (!PQsendQuery(conn, "show transaction_read_only")) + { + restoreErrorMessage(conn, &savedMessage); goto error_return; + } conn->status = CONNECTION_CHECK_WRITABLE; + restoreErrorMessage(conn, &savedMessage); return PGRES_POLLING_READING; } @@ -2841,11 +2890,18 @@ keep_going: /* We will come back to here until there is if (conn->target_session_attrs != NULL && strcmp(conn->target_session_attrs, "read-write") == 0) { + if (!saveErrorMessage(conn, &savedMessage)) + goto error_return; + conn->status = CONNECTION_OK; if (!PQsendQuery(conn, "show transaction_read_only")) + { + restoreErrorMessage(conn, &savedMessage); goto error_return; + } conn->status = CONNECTION_CHECK_WRITABLE; + restoreErrorMessage(conn, &savedMessage); return PGRES_POLLING_READING; } @@ -2858,13 +2914,20 @@ keep_going: /* We will come back to here until there is case CONNECTION_CHECK_WRITABLE: { + if (!saveErrorMessage(conn, &savedMessage)) + goto error_return; + conn->status = CONNECTION_OK; if (!PQconsumeInput(conn)) + { + restoreErrorMessage(conn, &savedMessage); goto error_return; + } if (PQisBusy(conn)) { conn->status = CONNECTION_CHECK_WRITABLE; + restoreErrorMessage(conn, &savedMessage); return PGRES_POLLING_READING; } @@ -2878,6 +2941,7 @@ keep_going: /* We will come back to here until there is if (strncmp(val, "on", 2) == 0) { PQclear(res); + restoreErrorMessage(conn, &savedMessage); /* Not writable; close connection. */ appendPQExpBuffer(&conn->errorMessage, @@ -2895,6 +2959,7 @@ keep_going: /* We will come back to here until there is if (conn->whichhost + 1 < conn->nconnhost) { conn->status = CONNECTION_NEEDED; + restoreErrorMessage(conn, &savedMessage); goto keep_going; } @@ -2902,6 +2967,7 @@ keep_going: /* We will come back to here until there is goto error_return; } PQclear(res); + termPQExpBuffer(&savedMessage); /* We can release the address lists now. */ release_all_addrinfo(conn); @@ -2917,6 +2983,7 @@ keep_going: /* We will come back to here until there is */ if (res) PQclear(res); + restoreErrorMessage(conn, &savedMessage); appendPQExpBuffer(&conn->errorMessage, libpq_gettext("test \"show transaction_read_only\" failed " " on \"%s:%s\" \n"),
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers