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(dot)korotkov(at)postgrespro(dot)ru
> <mailto:a(dot)korotkov(at)postgrespro(dot)ru>> wrote:
>
> On Tue, Mar 13, 2018 at 3:25 PM, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org
> <mailto:alvherre(at)alvh(dot)no-ip(dot)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(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/