Thread: BUG #15170: PQtransactionStatus returns ACTIVE after Empty Commit

The following bug has been logged on the website:

Bug reference:      15170
Logged by:          Rick Gabriel
Email address:      klaxian@gmail.com
PostgreSQL version: 10.3
Operating system:   Ubuntu Linux 16.04
Description:

PQtransactionStatus() incorrectly returns PQTRANS_ACTIVE after committing an
empty transaction block (ie. BEGIN and COMMIT without any other queries).
From the documentation, I would expect PQTRANS_IDLE to be returned in this
case since the connection becomes idle, outside a transaction. If no other
queries are executed on the connection, it never returns to PQTRANS_IDLE
even after waiting for several seconds. I am testing with the PHP 7.1
PostgreSQL module, but I think this is a universal problem with PostgreSQL's
API.

=?utf-8?q?PG_Bug_reporting_form?= <noreply@postgresql.org> writes:
> PQtransactionStatus() incorrectly returns PQTRANS_ACTIVE after committing an
> empty transaction block (ie. BEGIN and COMMIT without any other queries).

I do not see that here; for instance, the attached test program prints

Initial PQtransactionStatus: 0
PQtransactionStatus after BEGIN: 2
PQtransactionStatus after COMMIT: 0
PQtransactionStatus after BEGIN; COMMIT: 0

which is what I'd expect (0 is PQTRANS_IDLE, 2 is PQTRANS_INTRANS).
I'd only expect PQTRANS_ACTIVE if libpq thinks a query is still "in
flight", ie the server hasn't returned a command-complete message.

I speculate that either you or PHP have not waited for the commit
response to come back ... but without a concrete example to look at,
it's hard to say more.

            regards, tom lane

#include <stdio.h>
#include <stdlib.h>
#include "libpq-fe.h"

static void
exit_nicely(PGconn *conn)
{
    PQfinish(conn);
    exit(1);
}

int
main(int argc, char **argv)
{
    const char *conninfo;
    PGconn       *conn;
    PGresult   *res;

    if (argc > 1)
        conninfo = argv[1];
    else
        conninfo = "dbname = postgres user = postgres";

    /* Make a connection to the database */
    conn = PQconnectdb(conninfo);

    /* Check to see that the backend connection was successfully made */
    if (PQstatus(conn) != CONNECTION_OK)
    {
        fprintf(stderr, "Connection to database failed: %s",
                PQerrorMessage(conn));
        exit_nicely(conn);
    }

    printf("Initial PQtransactionStatus: %d\n",
           PQtransactionStatus(conn));

    res = PQexec(conn, "BEGIN");
    if (PQresultStatus(res) != PGRES_COMMAND_OK)
    {
        fprintf(stderr, "BEGIN failed: %s", PQerrorMessage(conn));
        PQclear(res);
        exit_nicely(conn);
    }
    PQclear(res);

    printf("PQtransactionStatus after BEGIN: %d\n",
           PQtransactionStatus(conn));

    res = PQexec(conn, "COMMIT");
    if (PQresultStatus(res) != PGRES_COMMAND_OK)
    {
        fprintf(stderr, "COMMIT failed: %s", PQerrorMessage(conn));
        PQclear(res);
        exit_nicely(conn);
    }
    PQclear(res);

    printf("PQtransactionStatus after COMMIT: %d\n",
           PQtransactionStatus(conn));

    res = PQexec(conn, "BEGIN; COMMIT");
    if (PQresultStatus(res) != PGRES_COMMAND_OK)
    {
        fprintf(stderr, "BEGIN; COMMIT failed: %s", PQerrorMessage(conn));
        PQclear(res);
        exit_nicely(conn);
    }
    PQclear(res);

    printf("PQtransactionStatus after BEGIN; COMMIT: %d\n",
           PQtransactionStatus(conn));

    /* close the connection to the database and cleanup */
    PQfinish(conn);

    return 0;
}

Rick Gabriel <klaxian@gmail.com> writes:
> First, I forgot to mention that I am sending async queries (PQsendQuery).
> libpq documentation states that PQgetResult() must be called repeatedly
> until it returns a null pointer.

Right.

> Unfortunately, there is nothing about this
> requirement in the official docs for PHP's libpq wrapper extension.

I think you have grounds to call that a PHP documentation bug.  Or at
least it should be made clearer that you also need to read the libpq docs.

> If I
> don't call PQgetResult() one more time than I really need, the transaction
> and connection statuses remain active/busy. Even though PG reports its
> status as "busy", subsequent queries succeed normally.

That's because if you start a new query, libpq will read-and-discard any
remaining server output from the last one.  That's not really a good thing
to depend on though --- for example, you might miss an error message that
way.

> I suggest that the connection and transaction states should be updated when
> all queued async queries are completed, without the extra call to
> PQgetResult().
> What do you think?

I think we're not going to change that, because

(a) it's not a bug, it's operating as designed and documented;
(b) this behavior has been stable for more than a dozen years, and
    changing it might well break clients expecting the current behavior;
(c) it's not very clear how libpq could do that at all.

I think what you're seeing is related to the fact that the server's
response to a query string involves one or more query results (each
containing some Data messages then a CommandComplete message), followed
by a ReadyForQuery message.  PQgetResult will return a PGresult when it's
consumed a CommandComplete message, but you have to call it again to get
libpq to consume the RFQ message --- and it's that message that causes
the PQtransactionStatus result to change.  Since RFQ bears the info about
whether the server is truly idle or just idle-in-transaction, libpq cannot
correctly update PQtransactionStatus without reading that message.

You could argue that libpq should "look ahead" after reading
CommandComplete to see if there's an RFQ already received, but I don't
think that could be made to work reliably.  You'd either have unwanted
blocking in some cases, or false negatives, anytime the RFQ wasn't
contained in the same TCP packet as the last CC.  I doubt people would
want to work with a spec that says "99% of the time PQtransactionStatus
will be updated by the last real PQgetResult call, but sometimes you
need one more call".

            regards, tom lane