Now, ITSM that predicate locks and conflict checks are placed right for now.
However, it would be good to add couple comments to gistdoinsert() whose would state why do we call CheckForSerializableConflictIn() in these particular places.
I also take a look at isolation tests. You made two separate test specs: one to verify that serialization failures do fire, and another to check there are no false positives.
I wonder if we could merge this two test specs into one, but use more variety of statements with different keys for both inserts and selects.
Please find the updated version of patch here. I have made suggested changes.
In general, patch looks good for me now. I just see some cosmetic issues.
/* + *Check for any r-w conflicts (in serialisation isolation level) + *just before we intend to modify the page + */ +CheckForSerializableConflictIn(r, NULL, stack->buffer); +/*
Formatting doesn't look good here. You've missed space after star sign in the comment. You also missed newline after CheckForSerializableConflictIn() call.
Also, you've long comment lines in predicate-gist.spec. Please, break long comments into multiple lines.
I have fixed formatting style. Please take a look at updated patch.