Hi all
This morning for the the umpteenth time I saw:
some error message: [blank here]
output from a libpq program.
That's because passing a NULL PGresult to PQgetResultErrorMessage() returns "". But a NULL PGresult is a normal result from PQexec when it fails to submit a query due to an invalid connection, when PQgetResult can't get a result from an invalid connection, etc.
E.g. this pattern:
res = PQexec(conn, "something");
if (PQstatus(res) != PGRES_TUPLES_OK)
{
report_an_error("some error message: %s",
PQresultErrorMessage(res));
}
... where "res" is NULL because the connection was invalid, so PQresultErrorMessage(res) emits the empty string.
As a result, using PQerrorMessage(conn) is actually better in most cases, despite the static error buffer issues. It'll do the right thing when the connection itself is bad. Alternately you land up with the pattern
res == NULL ? PQerrorMessage(conn) : PQresultErrorMessage(res)
I'm not quite sure what to do about this. Ideally PQresultErrorMessage() would take the PGconn* too, but it doesn't, and it's not too friendly to add an extra argument. Plus arguably they mean different things.
Maybe it's as simple as changing the docs to say that you should prefer PQerrorMessage() if you aren't using multiple PGresult s at a time, and mentioning the need to copy the error string.
But I'd kind of like to instead return a new non-null PGresult PGRES_RESULT_ERROR whenever we'd currently return a NULL PGresult due to a failure. Embed the error message into the PGresult, so PQresultErrorMessage() can fetch it. Because a PGresult needs to be owned by a PGconn and a NULL PGconn can't own anything, we'd instead return a pointer to a static const global PGresult with value PGRES_NO_CONNECTION if any function is passed a NULL PGconn*. That way it doesn't matter if it gets PQclear()ed or not. And PQclear() could test for (res == PGresult_no_connection) and not try to free it if found.
The main issue I see there is that existing code may expect a NULL PGresult and may test for (res == NULL) as an indicator of a query-sending failure from PQexec etc. So I suspect we'd need a libpq-global option to enable this behaviour for apps that are aware of it - we wouldn't want to add new function signature variants after all.
Similar changes would make sense for returning NULL when there are no result sets remaining after a PQsendQuery, and for returning NULL after row-by-row fetch mode runs out of rows.