From 00d4f2af8bd6a1b9db2f676cc76b64d98ace99fb Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 3 Feb 2014 21:30:02 -0500 Subject: Improve connection-failure error handling in contrib/postgres_fdw. postgres_fdw tended to say "unknown error" if it tried to execute a command on an already-dead connection, because some paths in libpq just return a null PGresult for such cases. Out-of-memory might result in that, too. To fix, pass the PGconn to pgfdw_report_error, and look at its PQerrorMessage() string if we can't get anything out of the PGresult. Also, fix the transaction-exit logic to reliably drop a dead connection. It was attempting to do that already, but it assumed that only connection cache entries with xact_depth > 0 needed to be examined. The folly in that is that if we fail while issuing START TRANSACTION, we'll not have bumped xact_depth. (At least for the case I was testing, this fix masks the other problem; but it still seems like a good idea to have the PGconn fallback logic.) Per investigation of bug #9087 from Craig Lucas. Backpatch to 9.3 where this code was introduced. --- contrib/postgres_fdw/postgres_fdw.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'contrib/postgres_fdw/postgres_fdw.c') diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index ae3ab000e1f..fde1ec13617 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -1040,7 +1040,7 @@ postgresReScanForeignScan(ForeignScanState *node) */ res = PQexec(fsstate->conn, sql); if (PQresultStatus(res) != PGRES_COMMAND_OK) - pgfdw_report_error(ERROR, res, true, sql); + pgfdw_report_error(ERROR, res, fsstate->conn, true, sql); PQclear(res); /* Now force a fresh FETCH. */ @@ -1374,7 +1374,7 @@ postgresExecForeignInsert(EState *estate, 0); if (PQresultStatus(res) != (fmstate->has_returning ? PGRES_TUPLES_OK : PGRES_COMMAND_OK)) - pgfdw_report_error(ERROR, res, true, fmstate->query); + pgfdw_report_error(ERROR, res, fmstate->conn, true, fmstate->query); /* Check number of rows affected, and fetch RETURNING tuple if any */ if (fmstate->has_returning) @@ -1444,7 +1444,7 @@ postgresExecForeignUpdate(EState *estate, 0); if (PQresultStatus(res) != (fmstate->has_returning ? PGRES_TUPLES_OK : PGRES_COMMAND_OK)) - pgfdw_report_error(ERROR, res, true, fmstate->query); + pgfdw_report_error(ERROR, res, fmstate->conn, true, fmstate->query); /* Check number of rows affected, and fetch RETURNING tuple if any */ if (fmstate->has_returning) @@ -1514,7 +1514,7 @@ postgresExecForeignDelete(EState *estate, 0); if (PQresultStatus(res) != (fmstate->has_returning ? PGRES_TUPLES_OK : PGRES_COMMAND_OK)) - pgfdw_report_error(ERROR, res, true, fmstate->query); + pgfdw_report_error(ERROR, res, fmstate->conn, true, fmstate->query); /* Check number of rows affected, and fetch RETURNING tuple if any */ if (fmstate->has_returning) @@ -1563,7 +1563,7 @@ postgresEndForeignModify(EState *estate, */ res = PQexec(fmstate->conn, sql); if (PQresultStatus(res) != PGRES_COMMAND_OK) - pgfdw_report_error(ERROR, res, true, sql); + pgfdw_report_error(ERROR, res, fmstate->conn, true, sql); PQclear(res); fmstate->p_name = NULL; } @@ -1800,7 +1800,7 @@ get_remote_estimate(const char *sql, PGconn *conn, */ res = PQexec(conn, sql); if (PQresultStatus(res) != PGRES_TUPLES_OK) - pgfdw_report_error(ERROR, res, false, sql); + pgfdw_report_error(ERROR, res, conn, false, sql); /* * Extract cost numbers for topmost plan node. Note we search for a @@ -1934,7 +1934,7 @@ create_cursor(ForeignScanState *node) res = PQexecParams(conn, buf.data, numParams, NULL, values, NULL, NULL, 0); if (PQresultStatus(res) != PGRES_COMMAND_OK) - pgfdw_report_error(ERROR, res, true, fsstate->query); + pgfdw_report_error(ERROR, res, conn, true, fsstate->query); PQclear(res); /* Mark the cursor as created, and show no tuples have been retrieved */ @@ -1985,7 +1985,7 @@ fetch_more_data(ForeignScanState *node) res = PQexec(conn, sql); /* On error, report the original query, not the FETCH. */ if (PQresultStatus(res) != PGRES_TUPLES_OK) - pgfdw_report_error(ERROR, res, false, fsstate->query); + pgfdw_report_error(ERROR, res, conn, false, fsstate->query); /* Convert the data into HeapTuples */ numrows = PQntuples(res); @@ -2091,7 +2091,7 @@ close_cursor(PGconn *conn, unsigned int cursor_number) */ res = PQexec(conn, sql); if (PQresultStatus(res) != PGRES_COMMAND_OK) - pgfdw_report_error(ERROR, res, true, sql); + pgfdw_report_error(ERROR, res, conn, true, sql); PQclear(res); } @@ -2128,7 +2128,7 @@ prepare_foreign_modify(PgFdwModifyState *fmstate) NULL); if (PQresultStatus(res) != PGRES_COMMAND_OK) - pgfdw_report_error(ERROR, res, true, fmstate->query); + pgfdw_report_error(ERROR, res, fmstate->conn, true, fmstate->query); PQclear(res); /* This action shows that the prepare has been done. */ @@ -2278,7 +2278,7 @@ postgresAnalyzeForeignTable(Relation relation, { res = PQexec(conn, sql.data); if (PQresultStatus(res) != PGRES_TUPLES_OK) - pgfdw_report_error(ERROR, res, false, sql.data); + pgfdw_report_error(ERROR, res, conn, false, sql.data); if (PQntuples(res) != 1 || PQnfields(res) != 1) elog(ERROR, "unexpected result from deparseAnalyzeSizeSql query"); @@ -2372,7 +2372,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel, { res = PQexec(conn, sql.data); if (PQresultStatus(res) != PGRES_COMMAND_OK) - pgfdw_report_error(ERROR, res, false, sql.data); + pgfdw_report_error(ERROR, res, conn, false, sql.data); PQclear(res); res = NULL; @@ -2403,7 +2403,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel, res = PQexec(conn, fetch_sql); /* On error, report the original query, not the FETCH. */ if (PQresultStatus(res) != PGRES_TUPLES_OK) - pgfdw_report_error(ERROR, res, false, sql.data); + pgfdw_report_error(ERROR, res, conn, false, sql.data); /* Process whatever we got. */ numrows = PQntuples(res); -- cgit v1.2.3