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.
