Re: [HACKERS] Patching dblink.c to avoid warning about open

[ reposted]

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Well, as I said in the patch email:
>
> >     The reported problem is that dblink_open/dblink_close() (for cursor
> >     reads) do a BEGIN/COMMIT regardless of the transaction state of the
> >     remote connection.  There was code in dblink.c to track the remote
> >     transaction state (rconn), but it was not being maintained or used.
>
> You should lose the remoteXactOpen flag entirely, in favor of just
> testing PQtransactionStatus() on-the-fly when necessary.  Simpler,
> more reliable, not notably slower.
>
> With that change, the separate remoteConn struct could be dropped
> altogether in favor of just using the PGconn pointer.  This would
> make things notationally simpler, and in fact perhaps allow undoing
> the bulk of the edits in your patch.  As-is I think the patch is
> pretty risky to apply during beta.

I have developed a dblink regression patch that tests for the bug.  It
opens a transaction in the client code, opens/closes a cursor, then
tries a DECLARE.  Without my patch, this fails because the
dblink_close() closes the transaction, but with my patch it succeeds.
Here is the test:

    -- test opening cursor in a transaction
    SELECT dblink_exec('myconn','BEGIN');

    -- an open transaction will prevent dblink_open() from opening its own
    SELECT dblink_open('myconn','rmt_foo_cursor','SELECT * FROM foo');

    -- this should not close the cursor because the client opened it
    SELECT dblink_close('myconn','rmt_foo_cursor');

    -- this should succeed because we have an open transaction
    SELECT dblink_exec('myconn','DECLARE xact_test CURSOR FOR SELECT * FROM foo');

    -- commit remote transaction
    SELECT dblink_exec('myconn','COMMIT');

Here is the failure reported by the current code:

    -- this should succeed because we have an open transaction
    SELECT dblink_exec('myconn','DECLARE xact_test CURSOR FOR SELECT * FROM foo');
    ERROR:  sql error
    DETAIL:  ERROR:  DECLARE CURSOR may only be used in transaction blocks

There was also a problem in that if two cursors were opened, the first
close would close the transaction.  I have fixed that code by changing
the xact variable in to a counter that keeps track of the number of
opened cursors and commits only when they are all closed.

Both the dblink.c patch and the regression patch are at:

    ftp://candle.pha.pa.us/pub/postgresql/mypatches

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073