Re: [HACKERS] GSoC 2017 : Patch for predicate locking in Gist index

Hi, Shubham!

On Wed, Nov 1, 2017 at 12:10 AM, Shubham Barai <shubhambaraiss@gmail.com>
wrote:

> On 9 October 2017 at 18:57, Alexander Korotkov <a.korotkov@postgrespro.ru>
> 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