Simon Riggs <simon@2ndQuadrant.com> writes:
> On Mon, Oct 3, 2011 at 7:09 AM, Marko Tiikkaja
>> Thanks, this one looks good to me. �Going to mark this patch as ready for
>> committer.
> I don't see any tests with this patch, so I personally won't be the
> committer on this just yet.
I've already taken it according to the commitfest app. There's a lot of
things I don't like stylistically, but they all seem fixable, and I'm
working through them now. The only actual bug I've found so far is a
race condition while setting MyProc->xmin (you can't do that separately
from verifying that the source transaction is still running, else
somebody else could see a global xmin that's gone backwards).
> Also, not sure why the snapshot id syntax has leading zeroes on first
> part of the number, but not on second part. It will still sort
> incorrectly if that's what we were trying to achieve. Either leave off
> the leading zeroes on first part of add them to second.
The first part is of fixed length, the second not so much. I'm not
wedded to the syntax but I don't see anything wrong with it either.
> I'm also concerned that we are adding this to the BEGIN statement as
> the only option.
Huh? The last version of the patch has it only as SET TRANSACTION
SNAPSHOT, which I think is the right way.
regards, tom lane