On 1 October 2017 at 01:47, Alexander Korotkov <a.korot...@postgrespro.ru> wrote:
> On Sat, Sep 30, 2017 at 6:12 PM, Shubham Barai <shubhambara...@gmail.com> > wrote: > >> I have made changes according to your suggestions. Please have a look at >> the updated patch. >> I am also considering your suggestions for my other patches also. But, I >> will need some time to >> make changes as I am currently busy doing my master's. >> > > I don't understand why sometimes you call PredicateLockPage() only when > fast update is off. For example: > > @@ -94,6 +101,9 @@ scanPostingTree(Relation index, GinScanEntry scanEntry, >> break; /* no more pages */ >> >> buffer = ginStepRight(buffer, index, GIN_SHARE); >> + >> + if (!GinGetUseFastUpdate(index)) >> + PredicateLockPage(index, BufferGetBlockNumber(buffer), snapshot); >> } >> >> UnlockReleaseBuffer(buffer); > > > But sometimes you call PredicateLockPage() unconditionally. > > @@ -131,6 +141,8 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack >> *stack, >> attnum = scanEntry->attnum; >> attr = btree->ginstate->origTupdesc->attrs[attnum - 1]; >> >> + PredicateLockPage(btree->index, stack->buffer, snapshot); >> + >> for (;;) >> { >> Page page; > > > As I understand, all page-level predicate locking should happen only when > fast update is off. > > Also, despite general idea is described in README-SSI, in-code comments > would be useful. > > Hi Alexander, Yes, page-level predicate locking should happen only when fast update is off. Actually, I forgot to put conditions in updated patch. Does everything else look ok ? Kind Regards, Shubham >