Re: [HACKERS] GSoC 2017: weekly progress reports (week 6)

Hi!

Patch seems good, but I found one bug in it, in fact, nobody
checks serializible conflict with fastupdate=on:
gininsert()
{
    if (GinGetUseFastUpdate())
    {
    /* two next lines are GinCheckForSerializableConflictIn() */
        if (!GinGetUseFastUpdate())
            CheckForSerializableConflictIn()
    }
}

I  changed to direct call CheckForSerializableConflictIn() (see attachment)

I'd like to see fastupdate=on in test too, now tests cover only case without 
fastupdate. Please, add them.

Shubham Barai wrote:
> 
> 
> On 16 March 2018 at 03:57, Alexander Korotkov <a.korotkov@postgrespro.ru 
> <mailto:a.korotkov@postgrespro.ru>> wrote:
> 
>     On Tue, Mar 13, 2018 at 3:25 PM, Alvaro Herrera <alvherre@alvh.no-ip.org
>     <mailto:alvherre@alvh.no-ip.org>> wrote:
> 
>         Alexander Korotkov wrote:
> 
>         > And what happen if somebody concurrently set (fastupdate = on)?
>         > Can we miss conflicts because of that?
> 
>         I think it'd be better to have that option require AccessExclusive lock,
>         so that it can never be changed concurrently with readers.  Seems to me
>         that penalizing every single read to cope with this case would be a bad
>         trade-off.
> 
> 
>     As Andrey Borodin mentioned, we already do.  Sorry for buzz :)
> 
> 
> 
> I have updated the patch based on suggestions.
> 
> Regards,
> Shubham

-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/