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? If so, earlier I thought we might not need this check as we are already using heap_tuple_needs_eventual_freeze(), but now again looking at it, it seems wise to check for dead items separately as those won't be covered by other check. > > + /* 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. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
