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

On 27 November 2017 at 13:17, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
wrote:

> Hi, Shubham!
>
> On Wed, Nov 1, 2017 at 12:10 AM, Shubham Barai <shubhambaraiss(at)gmail(dot)com>
> wrote:
>
>> On 9 October 2017 at 18:57, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)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.
>
>
I have fixed formatting style. Please take a look at updated patch.

Regards,
Shubham

>
>