Em sex., 12 de fev. de 2021 às 03:56, Kyotaro Horiguchi < horikyota....@gmail.com> escreveu:
> At Wed, 10 Feb 2021 20:12:38 -0300, Ranier Vilela <ranier...@gmail.com> > wrote in > > Hi, > > > > Per Coverity. > > > > If xid is a subtransaction, the setup of base snapshot on the top-level > > transaction, > > can be not optional, otherwise a Dereference null return value > > (NULL_RETURNS) can be raised. > > > > Patch suggestion to fix this. > > > > diff --git a/src/backend/replication/logical/reorderbuffer.c > > b/src/backend/replication/logical/reorderbuffer.c > > index 5a62ab8bbc..3c6a81f716 100644 > > --- a/src/backend/replication/logical/reorderbuffer.c > > +++ b/src/backend/replication/logical/reorderbuffer.c > > @@ -2993,8 +2993,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb, > > TransactionId xid, > > */ > > txn = ReorderBufferTXNByXid(rb, xid, true, &is_new, lsn, true); > > if (rbtxn_is_known_subxact(txn)) > > - txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, false, > > - NULL, InvalidXLogRecPtr, false); > > + txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, true, > > + NULL, InvalidXLogRecPtr, true); > > Assert(txn->base_snapshot == NULL); > > If the return from the first call is a subtransaction, the second call > always obtain the top transaction. If the top transaction actualy did > not exist, it's rather the correct behavior to cause SEGV, than > creating a bogus rbtxn. THus it is wrong to set create=true and > create_as_top=true. We could change the assertion like Assert (txn && > txn->base_snapshot) to make things clearer. > It does not make sense to generate a SEGV on purpose and assertion would not solve why this happens in a production environment. Better to report an error if the second call fails. What do you suggest as a description of the error? regards, Ranier Vilela
