At Wed, 10 Feb 2021 20:12:38 -0300, Ranier Vilela <ranier.vf@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?
