On Tue, Jun 7, 2016 at 10:10 PM, Masahiko Sawada <sawada.m...@gmail.com>
> 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

> +       /* 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

Reply via email to