On Mon, Jan 18, 2021 at 03:08:38PM +0900, Kyotaro Horiguchi wrote: > At Fri, 15 Jan 2021 20:38:16 -0800, Noah Misch <n...@leadboat.com> wrote in > > On Wed, Jan 13, 2021 at 04:07:05PM +0900, Kyotaro Horiguchi wrote: > > > --- a/src/include/utils/snapmgr.h > > > +++ b/src/include/utils/snapmgr.h > > > @@ -37,7 +37,7 @@ > > > */ > > > #define RelationAllowsEarlyPruning(rel) \ > > > ( \ > > > - RelationNeedsWAL(rel) \ > > > + RelationIsWalLogged(rel) \ > > > > I suspect this is user-visible for a scenario like: > > > > CREATE TABLE t AS SELECT ...; DELETE FROM t; > > -- ... time passes, rows of t are now eligible for early pruning ... > > BEGIN; > > ALTER TABLE t SET TABLESPACE something; -- start skipping WAL > > SELECT count(*) FROM t; > > > > After this patch, the SELECT would do early pruning, as it does in the > > absence > > of the ALTER TABLE. When pruning doesn't update the page LSN, > > TestForOldSnapshot() will be unable to detect that early pruning changed the > > query results. Hence, RelationAllowsEarlyPruning() must not change this > > way. > > Does that sound right? > > Mmm, maybe no. The pruning works on XID (or timestamp), not on LSN, so > it seems to work well even if pruning happened at the SELECT. > Conversely that should work after old_snapshot_threshold elapsed. > > Am I missing something?
I wrote the above based on the "PageGetLSN(page) > (snapshot)->lsn" check in TestForOldSnapshot(). If the LSN isn't important, what else explains RelationAllowsEarlyPruning() checking RelationNeedsWAL()?