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

Joe Conway wrote:
> Tom Lane wrote:
> > David Fetter <david@fetter.org> writes:
> >
> >>On Thu, Oct 06, 2005 at 10:38:54PM -0400, Bruce Momjian wrote:
> >>
> >>>I don't know if people want this for 8.1 or 8.2.
> >
> >>8.1, IMHO.  It's a bug fix. :)
> >
> > Not unless Joe Conway signs off on it ...
> >
>
> I had asked on the original simple patch, and I'll ask again now -- why
> is this needed? I don't have a clear understanding of the problem that
> this (or the earlier) patch is trying to solve. Without that, it is
> impossible to say whether it is a bug fix or feature.

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.

If a remote transactions has already been started by the user,
dblink_open is going to call BEGIN, which is going to fail because there
is already an open transaction, right?

Here is an crude example:

    test=> BEGIN;
    BEGIN
    test=> begin;
    WARNING:  there is already a transaction in progress
    BEGIN
    test=> SELECT 1;
     ?column?
    ----------
            1
    (1 row)

    test=> COMMIT;
    COMMIT
    test=> COMMIT;
    WARNING:  there is no transaction in progress
    COMMIT

The BEGIN and COMMIT are only a warning though, but the early COMMIT by
the dblink_close() is a serious issue.

--
  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