Greg Stark <stark@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
