On Wed, Feb 09, 2022 at 12:48:35PM +0100, Peter Eisentraut wrote: > On 08.02.22 13:55, Julien Rouhaud wrote: > > I'm just saying that without such a lock you can easily trigger the "cache > > lookup" error, and that's something that's supposed to happen with normal > > usage I think. So it should be a better message saying that the database > > has > > been concurrently dropped, or actually simply does not exist like it's done > > in > > AlterDatabaseOwner() for the same pattern: > > > > [...] > > tuple = systable_getnext(scan); > > if (!HeapTupleIsValid(tuple)) > > ereport(ERROR, > > (errcode(ERRCODE_UNDEFINED_DATABASE), > > errmsg("database \"%s\" does not exist", > > dbname))); > > [...] > > In my code, the existence of the database is checked by > > dboid = get_database_oid(stmt->dbname, false); > > This also issues an appropriate user-facing error message if the database > does not exist.
Yes but if you run a DROP DATABASE concurrently you will either get a "database does not exist" or "cache lookup failed" depending on whether the DROP is processed before or after the get_database_oid(). I agree that it's not worth trying to make it concurrent-drop safe, but I also thought that throwing plain elog(ERROR) should be avoided when reasonably doable. And in that situation we know it can happen in normal situation, so having a real error message looks like a cost-free improvement. Now if it's better to have a cache lookup error even in that situation just for safety or something ok, it's not like trying to refresh a db collation and having someone else dropping it at the same time is going to be a common practice anway. > The flow in AlterDatabaseOwner() is a bit different, it looks up the > pg_database tuple directly from the name. I think both are correct. My > code has been copied from the analogous code in AlterCollation(). I also think it would be better to have a "collation does not exist" in the syscache failure message, but same here dropping collation is probably even less frequent than dropping database, let alone while refreshing the collation version.