Thread: BRIN and PageIndexDeleteNoCompact
BRIN added a new function, PageIndexDeleteNoCompact(), which is like PageIndexMultiDelete but does not remove unused line pointers. However, all the callers pass it just a single offset. So the callers would really be more interested in efficiently squeezing out a single tuple from a page, like PageIndexTupleDelete() does, than a bulk operation. PageIndexDeleteNoCompact() is not optimal for the single item case. Note that PageIndexMultiDelete() first checks if if the number of removed items is small (<= 2), and just calls PageIndexTupleDelete in a loop in that case. How about replacing PageIndexDeleteNoCompact() with something more like PageIndexTupleDelete()? It ought to be both faster and simpler. PS. The comment above PageIndexDeleteNoCompact says that unused items at the end of the array are removed. But looking at the code, I don't see it doing that. - Heikki
Heikki Linnakangas wrote: > BRIN added a new function, PageIndexDeleteNoCompact(), which is like > PageIndexMultiDelete but does not remove unused line pointers. However, all > the callers pass it just a single offset. So the callers would really be > more interested in efficiently squeezing out a single tuple from a page, > like PageIndexTupleDelete() does, than a bulk operation. Yeah, I noticed this and yes I agree there's no harm in changing it to a single-tuple mode rather than bulk operation. > How about replacing PageIndexDeleteNoCompact() with something more like > PageIndexTupleDelete()? It ought to be both faster and simpler. No objection. Are you working on this, or do you intend me to? How relevant is this given your current PageRepairFragmentation work? I think it will cause hard merge conflicts if done right away; should it be attempted prior to the PRF patches, or after it's done? I assume that if you do it, you can do both things simultaneously ... > PS. The comment above PageIndexDeleteNoCompact says that unused items at the > end of the array are removed. But looking at the code, I don't see it doing > that. Hmm, it probably lost the capability during the development :-( -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 11/18/2014 09:46 PM, Alvaro Herrera wrote: > Heikki Linnakangas wrote: >> How about replacing PageIndexDeleteNoCompact() with something more like >> PageIndexTupleDelete()? It ought to be both faster and simpler. > > No objection. Are you working on this, or do you intend me to? You, please ;-). > How relevant is this given your current PageRepairFragmentation work? > I think it will cause hard merge conflicts if done right away; should it > be attempted prior to the PRF patches, or after it's done? I assume > that if you do it, you can do both things simultaneously ... In my PageRepairFragmentation patch, I did refactor the common part of PageIndexDeleteNoCompact to use the shared function. It'll look completely different after it's rewritten to look more like PageIndexTupleDelete, and won't have anything in common with PageRepairFragmentation anymore, and won't really be any easier for me to do as part of the PFR work. So don't wait for that, just go ahead and make the change, whenever suits you. - Heikki