On Sun, Apr 10, 2022 at 8:05 PM Amit Langote <amitlangot...@gmail.com> wrote:
> On Fri, Apr 8, 2022 at 8:45 PM Amit Langote <amitlangot...@gmail.com> > wrote: > > Most looked fine changes to me except a couple of typos, so I've > > adopted those into the attached new version, even though I know it's > > too late to try to apply it. > > > > + * XXX is it worth doing a bms_copy() on glob->minLockRelids if > > + * glob->containsInitialPruning is true?. I'm slighly worried that the > > + * Bitmapset could have a very long empty tail resulting in excessive > > + * looping during AcquireExecutorLocks(). > > + */ > > > > I guess I trust your instincts about bitmapset operation efficiency > > and what you've written here makes sense. It's typical for leaf > > partitions to have been appended toward the tail end of rtable and I'd > > imagine their indexes would be in the tail words of minLockRelids. If > > copying the bitmapset removes those useless words, I don't see why we > > shouldn't do that. So added: > > > > + /* > > + * It seems worth doing a bms_copy() on glob->minLockRelids if we > deleted > > + * bit from it just above to prevent empty tail bits resulting in > > + * inefficient looping during AcquireExecutorLocks(). > > + */ > > + if (glob->containsInitialPruning) > > + glob->minLockRelids = bms_copy(glob->minLockRelids) > > > > Not 100% about the comment I wrote. > > And the quoted code change missed a semicolon in the v14 that I > hurriedly sent on Friday. (Had apparently forgotten to `git add` the > hunk to fix that). > > Sending v15 that fixes that to keep the cfbot green for now. > > -- > Amit Langote > EDB: http://www.enterprisedb.com Hi, + /* RT index of the partitione table. */ partitione -> partitioned Cheers
