On Sat, Jul 16, 2016 at 7:02 AM, Andres Freund <and...@anarazel.de> wrote: > On 2016-07-13 23:06:07 -0700, Andres Freund wrote: >> > + /* Clear only the all-frozen bit on visibility map if needed */ >> > + if (PageIsAllVisible(BufferGetPage(buffer)) && >> > + VM_ALL_FROZEN(relation, block, &vmbuffer)) >> > + { >> > + visibilitymap_clear_extended(relation, block, vmbuffer, >> > + >> > VISIBILITYMAP_ALL_FROZEN); >> > + } >> > + >> >> FWIW, I don't think it's worth introducing visibilitymap_clear_extended. >> As this is a 9.6 only patch, i think it's better to change >> visibilitymap_clear's API. > > Besides that easily fixed issue, the code also has the significant issue > that it's only performing the the visibilitymap processing in the > BLK_NEEDS_REDO case. But that's not ok, because both in the BLK_RESTORED > and the BLK_DONE cases the visibilitymap isn't guaranteed (or even > likely in the former case) to have been updated. > > I think we have two choices how to deal with that: First, we can add a > new flags variable to xl_heap_lock similar to > xl_heap_insert/update/... and bump page magic, >
+1 for going in this way. This will keep us consistent with how clear the visibility info in other places like heap_xlog_update(). -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers