On Tue, Aug 27, 2019 at 10:59 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > > At first glance it seems > > like we need to capture PageGetLSN(page) while we have the lock, and > > then later pass that into TestForOldSnapshot() instead of the page. > > I'll look into that and write a patch, probably in a day or two. > > Hm, but surely we need to do other things to the page besides > TestForOldSnapshot? I was imagining that we'd collect the > RelationHasUnloggedIndex flag (or perhaps better, the > RelationAllowsEarlyPruning result) before attempting to lock > the page, and then pass it to TestForOldSnapshot.
OK I started writing a patch and realised there were a few ugly problems that I was about to report here... but now I wonder if, based on the comment for RelationHasUnloggedIndex(), we shouldn't just nuke all this code. We don't actually support unlogged indexes on permanent tables (there is no syntax to create them, and RelationHasUnloggedIndex() will never return true in practice because RelationNeedsWAL() will always return false first). This is a locking protocol violation and a performance pessimisation in support of a feature we don't have. If we add support for that in some future release, we can figure out how to do it properly then, no? -- Thomas Munro https://enterprisedb.com
