Greg Stark <st...@mit.edu> writes: > Simple Rebase I took a little bit of a look through these.
* I find 0001 a bit scary, specifically that it's decided it's okay to apply extract_autovac_opts, pgstat_fetch_stat_tabentry_ext, and especially relation_needs_vacanalyze to another session's temp table. How safe is that really? * Don't see much point in renaming checkTempNamespaceStatus. That doesn't make it not an ABI break. If we want to back-patch this we'll have to do something different than what you did here. * In 0002, I don't especially approve of what you've done with the relminmxid calculation --- that seems to move it out of "pure bug fix" territory and into "hmm, I wonder if this creates new bugs" territory. Also, skipping that update for non-temp tables immediately falsifies ResetVacStats' claimed charter of "resetting to the same values used when creating tables". Surely GetOldestMultiXactId isn't *that* expensive, especially compared to the costs of file truncation. I think you should just do GetOldestMultiXactId straight up, and maybe submit a separate performance-improvement patch to make it do the other thing (in both places) for temp tables. * I wonder if this is the best place for ResetVacStats --- it doesn't seem to be very close to the code it needs to stay in sync with. If there's no better place, perhaps adding cross- reference comments in both directions would be advisable. * 0003 says it's running temp.sql by itself to avoid interference from other sessions, but sadly that cannot avoid interference from background autovacuum/autoanalyze. I seriously doubt this patch would survive contact with the buildfarm. Do we actually need a new test case? It's not like the code won't get exercised without it --- we have plenty of temp table truncations, surely. regards, tom lane