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.

Reply via email to