On Fri, Aug 21, 2015 at 8:22 PM, Robert Haas <
robertmhaas@gmail.com> wrote:
>
> On Wed, Aug 19, 2015 at 2:53 AM, Amit Kapila <
amit.kapila16@gmail.com> wrote:
> > I think one case where the patch can impact is for aborted transactions.
> > In TransactionIdIsInProgress(), we check for aborted transactions before
> > consulting pg_subtrans whereas with patch it will consult pg_subtrans
> > without aborted transaction check. Now it could be better to first check
> > pg_subtrans if we found that the corresponding top transaction is
> > in-progress as that will save extra pg_clog lookup, but I just mentioned
> > because that is one difference that I could see with this patch.
> >
> > Another minor point is, I think we should modify function level comment
> > for XidInMVCCSnapshot() where it says that this applies to known-
> > committed XIDs which will no longer be true after this patch.
>
> But only if the snapshot has overflowed, right?
Yes.
> That should affect
> only a small minority of cases.
>
Agreed, but the effect would be non-negligible for such cases. One thing
I am wondering that is there any harm in calling TransactionIdDidAbort()
in slow path before calling SubTransGetTopmostTransaction(), that can
also maintain consistency of checks in both the functions?