From 484b9587370ecb0325bfc30ca435697f9f52acc6 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Thu, 16 May 2024 14:11:00 -0700 Subject: [PATCH] Fix documentation about DROP DATABASE FORCE process termination rights. Specifically, it terminates a background worker even if the caller couldn't terminate the background worker with pg_terminate_backend(). Commit 3a9b18b3095366cd0c4305441d426d04572d88c1 neglected to update this. Back-patch to v13, which introduced DROP DATABASE FORCE. Reviewed by Amit Kapila. Reported by Kirill Reshke. Discussion: https://postgr.es/m/20240429212756.60.nmisch@google.com --- doc/src/sgml/ref/drop_database.sgml | 12 +++++++----- src/backend/storage/ipc/procarray.c | 25 ++++++++++++++----------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/doc/src/sgml/ref/drop_database.sgml b/doc/src/sgml/ref/drop_database.sgml index ff01450ba7..55c52ae93b 100644 --- a/doc/src/sgml/ref/drop_database.sgml +++ b/doc/src/sgml/ref/drop_database.sgml @@ -79,12 +79,14 @@ DROP DATABASE [ IF EXISTS ] name [ It doesn't terminate if prepared transactions, active logical replication slots or subscriptions are present in the target database. + - This will fail if the current user has no permissions to terminate other - connections. Required permissions are the same as with - pg_terminate_backend, described in - . This will also fail if we - are not able to terminate connections. + This terminates background worker connections and connections that the + current user has permission to terminate + with pg_terminate_backend, described in + . If connections would remain, + this command will fail. diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 3c4da3c1f1..4cfb22222b 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -3808,8 +3808,8 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared) * The current backend is always ignored; it is caller's responsibility to * check whether the current backend uses the given DB, if it's important. * - * It doesn't allow to terminate the connections even if there is a one - * backend with the prepared transaction in the target database. + * If the target database has a prepared transaction or permissions checks + * fail for a connection, this fails without terminating anything. */ void TerminateOtherDBBackends(Oid databaseId) @@ -3854,14 +3854,19 @@ TerminateOtherDBBackends(Oid databaseId) ListCell *lc; /* - * Check whether we have the necessary rights to terminate other - * sessions. We don't terminate any session until we ensure that we - * have rights on all the sessions to be terminated. These checks are - * the same as we do in pg_terminate_backend. + * Permissions checks relax the pg_terminate_backend checks in two + * ways, both by omitting the !OidIsValid(proc->roleId) check: * - * In this case we don't raise some warnings - like "PID %d is not a - * PostgreSQL server process", because for us already finished session - * is not a problem. + * - Accept terminating autovacuum workers, since DROP DATABASE + * without FORCE terminates them. + * + * - Accept terminating bgworkers. For bgworker authors, it's + * convenient to be able to recommend FORCE if a worker is blocking + * DROP DATABASE unexpectedly. + * + * Unlike pg_terminate_backend, we don't raise some warnings - like + * "PID %d is not a PostgreSQL server process", because for us already + * finished session is not a problem. */ foreach(lc, pids) { @@ -3870,13 +3875,11 @@ TerminateOtherDBBackends(Oid databaseId) if (proc != NULL) { - /* Only allow superusers to signal superuser-owned backends. */ if (superuser_arg(proc->roleId) && !superuser()) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("must be a superuser to terminate superuser process"))); - /* Users can signal backends they have role membership in. */ if (!has_privs_of_role(GetUserId(), proc->roleId) && !has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_BACKEND)) ereport(ERROR,