From: | Merlin Moncure <mmoncure(at)gmail(dot)com> |
---|---|
To: | Mark Hills <Mark(dot)Hills(at)framestore(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: libpq: Return of NULL from PQexec |
Date: | 2011-09-12 16:50:41 |
Message-ID: | CAHyXU0ztPbJUuyk0KszPQJjN5BbFB2z+hgehT93CXLWoOn3rnw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Sep 12, 2011 at 10:40 AM, Mark Hills <Mark(dot)Hills(at)framestore(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | David Fetter | 2011-09-12 17:21:26 | Re: [BUGS] BUG #6189: libpq: sslmode=require verifies server certificate if root.crt is present |
Previous Message | Dimitri Fontaine | 2011-09-12 16:50:28 | Re: What Would You Like To Do? |