Hi, Shubham!
On Wed, Nov 1, 2017 at 12:10 AM, Shubham Barai
wrote:
> On 9 October 2017 at 18:57, Alexander Korotkov
> wrote:
>
>> 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.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
