1. In CheckForSerializableConflictIn(), I think the comment above may be
out of date. It says:
"A tuple insert is in conflict only if there is a predicate lock against
the entire relation."
That doesn't appear to be true if, for example, there's a predicate lock
on the index page that the tuple goes into. I examined it with gdb, and
it calls the function, and the function does identify the conflict.
2. Also in the comment above CheckForSerializableConflictIn(), I see:
"The call to this function also indicates that we need an entry in the
serializable transaction hash table, so that this write's conflicts can
be detected for the proper lifetime, which is until this transaction and
all overlapping serializable transactions have completed."
which doesn't make sense to me. The transaction should already have an
entry in the hash table at this point, right?
3. The comment above CheckForSerializableConflictOut() seems to trail
off, as though you may have meant to write more. It also seems to be out
of date.
And why are you reading the infomask directly? Do the existing
visibility functions not suffice?
I have made it through predicate.c, and I have a much better
understanding of what it's actually doing. I can't claim that I have a
clear understanding of everything involved, but the code looks like it's
in good shape (given the complexity involved) and well-commented.
I am marking the patch Ready For Committer, because any committer will
need time to go through the patch; and the authors have clearly applied
the thought, care, and testing required for something of this
complexity. I will continue to work on it, though.
The biggest issue on my mind is what to do about Hot Standby. The
authors have a plan, but there is also some resistance to it:
http://archives.postgresql.org/message-id/23698.1295566621@sss.pgh.pa.us
We don't need a perfect solution for 9.1, but it would be nice if we had
a viable plan for 9.2.
Regards,Jeff Davis
