On Mon, Nov 9, 2020 at 3:23 PM Greg Stark <st...@mit.edu> wrote: > > On Mon, 9 Nov 2020 at 00:17, Noah Misch <n...@leadboat.com> wrote: > > > > > 2) adding the dependency on heapam.h to heap.c makes sense because of > > > heap_inplace_update bt it may be a bit annoying because I suspect > > > that's a useful sanity check that the tableam stuff hasn't been > > > bypassed > > > > That is not terrible. How plausible would it be to call > > vac_update_relstats() > > for this, instead of reimplementing part of it? > > It didn't seem worth it to change its API to add boolean flags to skip > setting some of the variables (I was originally only doing > relfrozenxid and minmmxid). Now that I'm doing most of the variables > maybe it makes a bit more sense. > > > > @@ -3340,6 +3383,7 @@ heap_truncate_one_rel(Relation rel) > > > > > > /* Truncate the underlying relation */ > > > table_relation_nontransactional_truncate(rel); > > > + ResetVacStats(rel); > > > > I didn't test, but I expect this will cause a stats reset for the second > > TRUNCATE here: > > > > CREATE TABLE t (); > > ... > > BEGIN; > > TRUNCATE t; > > TRUNCATE t; -- inplace relfrozenxid reset > > ROLLBACK; -- inplace reset survives > > > > Does that indeed happen? > > Apparently no, see below. I have to say I was pretty puzzled by the > actual behaviour which is that the rollback actually does roll back > the inplace update. But I *think* what is happening is that the first > truncate does an MVCC update so the inplace update happens only to the > newly created tuple which is never commited.
I think in-plase update that the patch introduces is not used because TRUNCATE doesn't use heap_truncate_one_rel() to truncate a table in that scenario. It does MVCC update the pg_class tuple for a new relfilenode with new relfrozenxid and other stats, see RelationSetNewRelfilenode(). If we create and truncate a table within the transaction it does in-place update that the patch introduces but I think it's no problem in this case either. > > Thinking about things a bit this does worry me a bit. I wonder if > inplace update is really safe outside of vacuum where we know we're > not in a transaction that can be rolled back. But IIRC doing a > non-inplace update on pg_class for these columns breaks other things. > I don't know if that's still true. heap_truncate_one_rel() is not a transaction-safe operation. Doing in-place updates during that operation seems okay to me unless I'm missing something. Regards, -- Masahiko Sawada EnterpriseDB: https://www.enterprisedb.com/