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

Joe Conway <mail@joeconway.com> writes:
> Here is my counter-proposal to Bruce's dblink patch. Any comments?

Minor coding suggestion: to me it seems messy to do

> +     int       *openCursorCount = NULL;
> +     bool       *newXactForCursor = NULL;

> !         openCursorCount = &pconn->openCursorCount;
> !         newXactForCursor = &pconn->newXactForCursor;

> !         *newXactForCursor = TRUE;

This looks a bit cluttered already, and would get more so if you need to
add more fields to a remoteConn.  Plus it confuses the reader (at least
this reader) who is left wondering if you intend that those variables
might sometimes point to something other than two fields of the same
remoteConn.  I think it would be shorter and clearer to write

    remoteConn  *remconn = NULL;
    ...
    remconn = rconn;
    ...
    remconn->newXactForCursor = TRUE;

Also, you might be able to combine this variable with the existing
rconn local variable and thus simplify the code even more.

> Is it too late to apply this for 8.1? I tend to agree with calling this
> a bugfix.

I think it's reasonable to fix now, yes.

            regards, tom lane