On Wed, Jun 8, 2016 at 12:19 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Tue, Jun 7, 2016 at 10:10 PM, Masahiko Sawada <sawada.m...@gmail.com> > wrote: >> >> Thank you for implementing the patch. >> >> I've not test it deeply but here are some comments. >> This check tool only checks if the frozen page has live-unfrozen tuple. >> That is, it doesn't care in case where the all-frozen page mistakenly >> has dead-frozen tuple. >> > > Do you mean to say that we should have a check for ItemIdIsDead() and then > if item is found to be dead, then add it to array of non_frozen items?
Yes. > If so, earlier I thought we might not need this check as we are already using > heap_tuple_needs_eventual_freeze(), You're right. Sorry, I had misunderstood. > but now again looking at it, it seems > wise to check for dead items separately as those won't be covered by other > check. Sounds good. >> >> + /* Clean up. */ >> + if (vmbuffer != InvalidBuffer) >> + ReleaseBuffer(vmbuffer); >> >> I think that we should use BufferIsValid() here. >> > > We can use BufferIsValid() as well, but I am trying to be consistent with > nearby code, refer collect_visibility_data(). We can change at all places > together if people prefer that way. > In vacuumlazy.c we use it like BufferisValid(vmbuffer), so I think we can replace all these thing to be more safety if there is not specific reason. Regards, -- Masahiko Sawada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
