> 27 марта 2018 г., в 13:45, Alexander Korotkov <a.korot...@postgrespro.ru> > написал(а): > > On Tue, Mar 27, 2018 at 11:16 AM, Andrey Borodin <x4...@yandex-team.ru > <mailto:x4...@yandex-team.ru>> wrote: > > 27 марта 2018 г., в 12:53, Teodor Sigaev <teo...@sigaev.ru > > <mailto:teo...@sigaev.ru>> написал(а): > > > > I have a question: why do not CheckForSerializableConflictIn() move into > > begining of gistplacetopage()? Seems, it is the single function which > > actually changes page and all predicate locking stuff will be placed in > > single function... > > gistplacetopage() is called from > 1. Buffered build - probably harmless > > Yes, harmless, but useless. > > 2. Finish split - i'm not sure about this. It seems to me that it is > necessary... then your version is correct. > > Yes, it's necessary, because GiST scan can end up on non-leaf page. So, scan > and modify of same non-leaf page should conflict. > > Checking for serializable conflicts from buffering build seems useless > overhead. gistplacetopage() > is called from only two places: gistinserttuples() and > gistbufferinginserttuples(). In order to evade > useless overhead for buffering build, I've moved > CheckForSerializableConflictIn() into gistinserttuples(). +1 Also, both gistdoinsert() and gistplacetopage() are mind blowing in complexity. gistinserttuples() looks like a cosy place if we need to add some more.
Best regards, Andrey Borodin.
