On 12/21/2016 04:22 PM, Tom Lane wrote: > Joe Conway <m...@joeconway.com> writes: >> I did notice that postgres_fdw has the following stanza that I don't see >> in dblink: > >> 8<------------------ >> /* >> * If we don't get a message from the PGresult, try the PGconn. This >> * is needed because for connection-level failures, PQexec may just >> * return NULL, not a PGresult at all. >> */ >> if (message_primary == NULL) >> message_primary = PQerrorMessage(conn); >> 8<------------------ > >> I wonder if the original issue on pgsql-bugs was a connection-level >> failure rather than OOM? Seems like dblink ought to do the same. > > Oooh ... I had thought that code was in both, which was why I was having > a hard time explaining the OP's failure. But I see you're right, > which provides a very straightforward explanation for the report. > I believe that if libpq is unable to malloc a PGresult, it will return > NULL but put an "out of memory" message into the PGconn's error buffer. > I had supposed that we'd capture and report the latter, but as the > dblink code stands, it won't. > > In short, yes, please copy that bit into dblink.
The attached should do the trick I think. You think it is reasonable to backpatch this part too? Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index ee45cd2..db3085a 100644 *** a/contrib/dblink/dblink.c --- b/contrib/dblink/dblink.c *************** static Relation get_rel_from_relname(tex *** 112,118 **** static char *generate_relation_name(Relation rel); static void dblink_connstr_check(const char *connstr); static void dblink_security_check(PGconn *conn, remoteConn *rconn); ! static void dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail); static char *get_connect_string(const char *servername); static char *escape_param_str(const char *from); static void validate_pkattnums(Relation rel, --- 112,119 ---- static char *generate_relation_name(Relation rel); static void dblink_connstr_check(const char *connstr); static void dblink_security_check(PGconn *conn, remoteConn *rconn); ! static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res, ! const char *dblink_context_msg, bool fail); static char *get_connect_string(const char *servername); static char *escape_param_str(const char *from); static void validate_pkattnums(Relation rel, *************** dblink_open(PG_FUNCTION_ARGS) *** 427,433 **** res = PQexec(conn, buf.data); if (!res || PQresultStatus(res) != PGRES_COMMAND_OK) { ! dblink_res_error(conname, res, "could not open cursor", fail); PG_RETURN_TEXT_P(cstring_to_text("ERROR")); } --- 428,434 ---- res = PQexec(conn, buf.data); if (!res || PQresultStatus(res) != PGRES_COMMAND_OK) { ! dblink_res_error(conn, conname, res, "could not open cursor", fail); PG_RETURN_TEXT_P(cstring_to_text("ERROR")); } *************** dblink_close(PG_FUNCTION_ARGS) *** 496,502 **** res = PQexec(conn, buf.data); if (!res || PQresultStatus(res) != PGRES_COMMAND_OK) { ! dblink_res_error(conname, res, "could not close cursor", fail); PG_RETURN_TEXT_P(cstring_to_text("ERROR")); } --- 497,503 ---- res = PQexec(conn, buf.data); if (!res || PQresultStatus(res) != PGRES_COMMAND_OK) { ! dblink_res_error(conn, conname, res, "could not close cursor", fail); PG_RETURN_TEXT_P(cstring_to_text("ERROR")); } *************** dblink_fetch(PG_FUNCTION_ARGS) *** 599,605 **** (PQresultStatus(res) != PGRES_COMMAND_OK && PQresultStatus(res) != PGRES_TUPLES_OK)) { ! dblink_res_error(conname, res, "could not fetch from cursor", fail); return (Datum) 0; } else if (PQresultStatus(res) == PGRES_COMMAND_OK) --- 600,607 ---- (PQresultStatus(res) != PGRES_COMMAND_OK && PQresultStatus(res) != PGRES_TUPLES_OK)) { ! dblink_res_error(conn, conname, res, ! "could not fetch from cursor", fail); return (Datum) 0; } else if (PQresultStatus(res) == PGRES_COMMAND_OK) *************** dblink_record_internal(FunctionCallInfo *** 750,757 **** if (PQresultStatus(res) != PGRES_COMMAND_OK && PQresultStatus(res) != PGRES_TUPLES_OK) { ! dblink_res_error(conname, res, "could not execute query", ! fail); /* if fail isn't set, we'll return an empty query result */ } else --- 752,759 ---- if (PQresultStatus(res) != PGRES_COMMAND_OK && PQresultStatus(res) != PGRES_TUPLES_OK) { ! dblink_res_error(conn, conname, res, ! "could not execute query", fail); /* if fail isn't set, we'll return an empty query result */ } else *************** materializeQueryResult(FunctionCallInfo *** 996,1002 **** PGresult *res1 = res; res = NULL; ! dblink_res_error(conname, res1, "could not execute query", fail); /* if fail isn't set, we'll return an empty query result */ } else if (PQresultStatus(res) == PGRES_COMMAND_OK) --- 998,1005 ---- PGresult *res1 = res; res = NULL; ! dblink_res_error(conn, conname, res1, ! "could not execute query", fail); /* if fail isn't set, we'll return an empty query result */ } else if (PQresultStatus(res) == PGRES_COMMAND_OK) *************** dblink_exec(PG_FUNCTION_ARGS) *** 1431,1437 **** (PQresultStatus(res) != PGRES_COMMAND_OK && PQresultStatus(res) != PGRES_TUPLES_OK)) { ! dblink_res_error(conname, res, "could not execute command", fail); /* * and save a copy of the command status string to return as our --- 1434,1441 ---- (PQresultStatus(res) != PGRES_COMMAND_OK && PQresultStatus(res) != PGRES_TUPLES_OK)) { ! dblink_res_error(conn, conname, res, ! "could not execute command", fail); /* * and save a copy of the command status string to return as our *************** dblink_connstr_check(const char *connstr *** 2662,2668 **** } static void ! dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail) { int level; char *pg_diag_sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE); --- 2666,2673 ---- } static void ! dblink_res_error(PGconn *conn, const char *conname, PGresult *res, ! const char *dblink_context_msg, bool fail) { int level; char *pg_diag_sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE); *************** dblink_res_error(const char *conname, PG *** 2696,2701 **** --- 2701,2714 ---- xpstrdup(message_hint, pg_diag_message_hint); xpstrdup(message_context, pg_diag_context); + /* + * If we don't get a message from the PGresult, try the PGconn. This + * is needed because for connection-level failures, PQexec may just + * return NULL, not a PGresult at all. + */ + if (message_primary == NULL) + message_primary = PQerrorMessage(conn); + if (res) PQclear(res);
signature.asc
Description: OpenPGP digital signature