Re: Slightly bogus regression test for contrib/dblink

Tom Lane wrote:
> Lines 509-512 of contrib/dblink/expected/dblink.out read:
>
> -- this should fail because there is no open transaction
> SELECT dblink_exec('myconn','DECLARE xact_test CURSOR FOR SELECT * FROM foo');
> ERROR: sql error
> DETAIL: ERROR: cursor "xact_test" already exists
>
> The error message is not consistent with what the comment claims.
> Looking at the test case in total, I think the code is responding

Actually the comment was correct, but there was a bug which caused the
automatically opened transaction to not get closed.

I was really expecting a "DECLARE CURSOR may only be used in transaction
blocks" error there, but didn't notice that I was actually getting a
different error message :-(.

The bug can be reproduced by using dblink_open to automatically open a
transaction, and then using dblink_exec to manually ABORT it:

-- open a test connection
SELECT dblink_connect('myconn','dbname=contrib_regression');
dblink_connect
----------------
OK
(1 row)

-- open a cursor incorrectly; this bumps up the refcount
contrib_regression=# SELECT
dblink_open('myconn','rmt_foo_cursor','SELECT * FROM foobar',false);
NOTICE: sql error
DETAIL: ERROR: relation "foobar" does not exist

dblink_open
-------------
ERROR
(1 row)

-- manually abort remote transaction; does not maintain refcount
SELECT dblink_exec('myconn','ABORT');
dblink_exec
-------------
ROLLBACK
(1 row)

-- test automatic transaction; this bumps up the refcount
SELECT dblink_open('myconn','rmt_foo_cursor','SELECT * FROM foo');
dblink_open
-------------
OK
(1 row)

-- this should commit the automatically opened transaction
-- but it doesn't because the refcount is wrong
SELECT dblink_close('myconn','rmt_foo_cursor');
dblink_close
--------------
OK
(1 row)

-- this should fail because there is no open transaction
-- but it doesn't because dblink_close did not commit
SELECT dblink_exec('myconn','DECLARE xact_test2 CURSOR FOR SELECT * FROM
foo');
dblink_exec
----------------
DECLARE CURSOR
(1 row)

I think the attached patch does the trick in a minimally invasive way.
If there are no objections I'll commit to head and 8.1 stable branches.
The problem doesn't exist before 8.1.

> BTW, I was led to notice this while examining the current buildfarm
> failure report from osprey,
> http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=osprey&dt=2006-06-17%2004:00:16
> It looks to me like the diffs are consistent with the idea that the test
> is using a copy of dblink that predates this patch ... do you agree?
> If so, anyone have an idea how that could happen? I thought we'd fixed
> all the rpath problems, and anyway osprey wasn't failing like this
> before today.

I haven't really looked at the buildfarm before -- I might be blind, but
I couldn't figure out how to see the regression.diff file.

Joe