On Sat, 25 Oct 2025 at 04:08, Nathan Bossart <nathandbossart@gmail.com> wrote:
> Here is an updated patch based on the latest discussion.
Thanks. I've just had a look at it. A few comments and questions.
1) The subtraction here looks back to front:
+ xid_age = TransactionIdIsNormal(relfrozenxid) ? relfrozenxid - recentXid : 0;
+ mxid_age = MultiXactIdIsValid(relminmxid) ? relminmxid - recentMulti : 0;
2) Would it be better to move all the code that sets the xid_score and
mxid_score to under an "if (force_vacuum)"? Those two variables could
be declared in there too.
3) Could the following be refactored a bit so we only check the "relid
!= StatisticRelationId" condition once?
+ if (relid != StatisticRelationId &&
+ classForm->relkind != RELKIND_TOASTVALUE)
Something like:
/* ANALYZE refuses to work with pg_statistic and we don't analyze
toast tables */
if (anltuples > anlthresh && relid != StatisticRelationId &&
classForm->relkind != RELKIND_TOASTVALUE)
{
*doanalyze = true;
// calc analyze score and Max with *score
}
else
*doanalyze = false;
then delete:
/* ANALYZE refuses to work with pg_statistic */
if (relid == StatisticRelationId)
*doanalyze = false;
4) Should these be TransactionIds?
+ uint32 xid_age;
+ uint32 mxid_age;
5) Instead of:
+ double score = 0.0;
Is it better to zero the score inside relation_needs_vacanalyze() so
it works the same as the other output parameters?
David