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

[ reposted]

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073