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/