Thread: IsTransactionState() is being used incorrectly

I just noticed that there are a number of places (mostly GUC assignment
hooks) that use IsTransactionState() to decide if it's safe for them to
do catalog lookups.  This seems pretty bogus because IsTransactionState
will return true in an aborted transaction.  I'm not sure there's any
actual bug because of other constraints on when GUC updates occur, but
it sure looks like trouble waiting to happen.

We could fix this either by changing the definition of
IsTransactionState() or by introducing another test function with
a different name.  Any thoughts which is preferable?
        regards, tom lane

Is this done or should it be kept for 8.4?

---------------------------------------------------------------------------

Tom Lane wrote:
> I just noticed that there are a number of places (mostly GUC assignment
> hooks) that use IsTransactionState() to decide if it's safe for them to
> do catalog lookups.  This seems pretty bogus because IsTransactionState
> will return true in an aborted transaction.  I'm not sure there's any
> actual bug because of other constraints on when GUC updates occur, but
> it sure looks like trouble waiting to happen.
> 
> We could fix this either by changing the definition of
> IsTransactionState() or by introducing another test function with
> a different name.  Any thoughts which is preferable?
> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster

--  Bruce Momjian  <bruce@momjian.us>          http://momjian.us EnterpriseDB
http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> We could fix this either by changing the definition of
>> IsTransactionState() or by introducing another test function with
>> a different name.  Any thoughts which is preferable?

> Is this done or should it be kept for 8.4?

Fixed, I thought ... yeah, here:

2007-06-07 17:45  tgl
* src/backend/: access/transam/xact.c, storage/ipc/procarray.c,utils/error/elog.c: Redefine IsTransactionState() to
onlyreturntrue for TRANS_INPROGRESS state, which is the only state in whichit's safe to initiate database queries.  It
turnsout that all buttwo of the callers thought that's what it meant; and the other twowere using it as a proxy for
"willGetTopTransactionId() return anonzero XID"?  Since it was in fact an unreliable guide to that,make those two just
invokeGetTopTransactionId() always, then dealwith a zero result if they get one.
 
        regards, tom lane