On Wed, Nov 18, 2020 at 06:32:51AM +0000, Junfeng Yang wrote:
> A path is attached co auther by Ashwin Agrawal, the solution is to
> fetch the pg_database tuple from disk instead of system cache if
> needed.
Yeah, we had better fix and I guess backpatch something here. That's
annoying.
+DROP DATABASE IF EXISTS vacuum_freeze_test;
+NOTICE: database "vacuum_freeze_test" does not exist, skipping
+CREATE DATABASE vacuum_freeze_test;
This test is costly. Extracting that into a TAP test would be more
adapted, still I am not really convinced that this is a case worth
spending extra cycles on.
+ tuple = systable_getnext(sscan);
+ if (HeapTupleIsValid(tuple))
+ tuple = heap_copytuple(tuple);
That's an unsafe pattern as the tuple scanned could finish by being
invalid.
One thing that strikes me as unwise is that we could run into similar
problems with vac_update_relstats() in the future, and there have been
recent talks about having more toast tables like for pg_class. That
is not worth caring about on stable branches because it is not an
active problem there, but we could do something better on HEAD.
+ /* Get the pg_database tuple to scribble on */
+ cached_tuple = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId));
+ cached_dbform = (Form_pg_database) GETSTRUCT(cached_tuple);
Er, shouldn't we *not* use the system cache at all here then? Or am I
missing something obvious? Please see the attached, that is more
simpleg.
--
Michael