Re: vac_update_datfrozenxid will raise "wrong tuple length" if pg_database tuple contains toast attribute.

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