Hi,
I'm hacking on this bugfix again, thanks to Evgeny's reminder on the other
thread [1].
I've been adding checks for partiall-dropped databases to the following places
so far:
- vac_truncate_clog(), as autovacuum can't process it anymore. Otherwise a
partially dropped database could easily lead to shutdown-due-to-wraparound.
- get_database_list() - so autovacuum workers don't error out when connecting
- template database used by CREATE DATABASE
- pg_dumpall, so we don't try to connect to the database
- vacuumdb, clusterdb, reindexdb, same
It's somewhat annoying that there is no shared place for the relevant query
for the client-side cases.
I haven't yet added checks to pg_upgrade, even though that's clearly
needed. I'm waffling a bit between erroring out and just ignoring the
database? pg_upgrade already fails when datallowconn is set "wrongly", see
check_proper_datallowconn(). Any opinions?
I'm not sure what should be done for psql. It's probably not a good idea to
change tab completion, that'd just make it appear the database is gone. But \l
could probably show dropped databases more prominently?
We don't really have a good place to for database specific
code. dbcommands.[ch] are for commands (duh), but already contain a bunch of
functions that don't really belong there. Seems we should add a
catalog/pg_database.c or catalog/database.c (tbh, I don't really know which we
use for what). But that's probably for HEAD only.
dbcommands.c's get_db_info() seems to have gone completely off the deep
end. It returns information in 14 separate out parameters, and the variables
for that need to all exhaustively be declared. And of course that differs
heavily between releases, making it a pain to backpatch any change. ISTM we
should just define a struct for the parameters - alternatively we could just
return a copy of the pg_database tuple, but it looks like the variable-width
attributes would make that *just* about a loss.
I guess that's once more something better dealt with on HEAD, but damn, I'm
not relishing having to deal with backpatching anything touching it - I think
it might be reasonable to just open-code fetching datconnlimit :/.
This patch is starting to be a bit big, particularly once adding tests for all
the checks mentioned above - but I haven't heard of or thought of a better
proposal :(.
Greetings,
Andres Freund
[1] https://postgr.es/m/01020188d31d0a86-16af92c0-4466-4cb6-a2e8-0e5898aab800-000000%40eu-west-1.amazonses.com