Thread: libpq: Return of NULL from PQexec

The libpq documentation for PQexec states:
 "If a null pointer is returned, it should be treated like a   PGRES_FATAL_ERROR result"

But this contradicts the example programs; eg. from Example Program 1:
  /* Start a transaction block */  res = PQexec(conn, "BEGIN");  if (PQresultStatus(res) != PGRES_COMMAND_OK)  {
fprintf(stderr,"BEGIN command failed: %s", PQerrorMessage(conn));      PQclear(res);      exit_nicely(conn);  }
 

which does not check if (res != NULL).

The example is not buggy -- PQresultStatus, PQerrorMessage and, 
importantly, PQclear deal with the NULL value; eg. src/libpq/fq-exec.c:
 ExecStatusType PQresultStatus(const PGresult *res) {     if (!res)         return PGRES_FATAL_ERROR;     return
res->resultStatus;}
 

So I took the example to be correct, and attempted to fix the 
documentation in the patch below. I hope this is useful.

In a straw-poll of code I could find using libpq, I found most follows the 
example and is reliant on libpq for its NULL check.

The same also applies to PQconnect functions -- and PQstatus, PQfinish, 
which I also tried to clarify in the documentation.

Thanks

-- 
Mark


diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 163a893..57be7e1 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -62,7 +62,7 @@   return a non-null object pointer, unless perhaps there is too   little memory even to allocate the
<structname>PGconn</>object.   The <function>PQstatus</> function should be called to check
 
-   whether a connection was successfully made before queries are sent
+   the return value for a successful connection before queries are sent   via the connection object.   <warning>
@@ -1748,8 +1748,10 @@ PGresult *PQexec(PGconn *conn, const char *command);        Returns a
<structname>PGresult</structname>pointer or possibly a null        pointer.  A non-null pointer will generally be
returnedexcept in        out-of-memory conditions or serious errors such as inability to send
 
-        the command to the server.  If a null pointer is returned, it should
-        be treated like a <symbol>PGRES_FATAL_ERROR</symbol> result.  Use
+        the command to the server.  The <function>PQresultStatus</> function
+        should be called to check the return value for any errors (including
+        the value of a null pointer, in which case it will return
+        <symbol>PGRES_FATAL_ERROR</symbol>).  Use        <function>PQerrorMessage</function> to get more information
aboutsuch        errors.       </para>
 

On Mon, Sep 12, 2011 at 10:40 AM, Mark Hills <Mark.Hills@framestore.com> wrote:
> The libpq documentation for PQexec states:
>
>  "If a null pointer is returned, it should be treated like a
>   PGRES_FATAL_ERROR result"
>
> But this contradicts the example programs; eg. from Example Program 1:
>
>   /* Start a transaction block */
>   res = PQexec(conn, "BEGIN");
>   if (PQresultStatus(res) != PGRES_COMMAND_OK)
>   {
>       fprintf(stderr, "BEGIN command failed: %s", PQerrorMessage(conn));
>       PQclear(res);
>       exit_nicely(conn);
>   }
>
> which does not check if (res != NULL).
>
> The example is not buggy -- PQresultStatus, PQerrorMessage and,
> importantly, PQclear deal with the NULL value; eg. src/libpq/fq-exec.c:
>
>  ExecStatusType
>  PQresultStatus(const PGresult *res)
>  {
>      if (!res)
>          return PGRES_FATAL_ERROR;
>      return res->resultStatus;
>  }
>
> So I took the example to be correct, and attempted to fix the
> documentation in the patch below. I hope this is useful.
>
> In a straw-poll of code I could find using libpq, I found most follows the
> example and is reliant on libpq for its NULL check.
>
> The same also applies to PQconnect functions -- and PQstatus, PQfinish,
> which I also tried to clarify in the documentation.
>
> Thanks
>
> --
> Mark
>
>
> diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
> index 163a893..57be7e1 100644
> --- a/doc/src/sgml/libpq.sgml
> +++ b/doc/src/sgml/libpq.sgml
> @@ -62,7 +62,7 @@
>    return a non-null object pointer, unless perhaps there is too
>    little memory even to allocate the <structname>PGconn</> object.
>    The <function>PQstatus</> function should be called to check
> -   whether a connection was successfully made before queries are sent
> +   the return value for a successful connection before queries are sent
>    via the connection object.
>
>    <warning>
> @@ -1748,8 +1748,10 @@ PGresult *PQexec(PGconn *conn, const char *command);
>         Returns a <structname>PGresult</structname> pointer or possibly a null
>         pointer.  A non-null pointer will generally be returned except in
>         out-of-memory conditions or serious errors such as inability to send
> -        the command to the server.  If a null pointer is returned, it should
> -        be treated like a <symbol>PGRES_FATAL_ERROR</symbol> result.  Use
> +        the command to the server.  The <function>PQresultStatus</> function
> +        should be called to check the return value for any errors (including
> +        the value of a null pointer, in which case it will return
> +        <symbol>PGRES_FATAL_ERROR</symbol>).  Use
>         <function>PQerrorMessage</function> to get more information about such
>         errors.
>        </para>

yeah -- libpq's handling of errors and result state is (put
charitably) pretty clunky -- a modernized api would probably lean on
thread local storage for global error state and return sane errors in
OOM conditions.  libpq simply demands wrapping if you want a clean
api.  anyways, +1 on the patch and the rationale -- the idea is to not
manually check NULL but to try and rely on the API -- this removes
(stupid) logic from userland.

merlin