Re: TerminateOtherDBBackends code comments inconsistency.

On Thu, Apr 11, 2024 at 6:58 PM Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
>
> While working on [0] i have noticed this comment in
> TerminateOtherDBBackends function:
>
> /*
> * 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.
> *
> * 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.
> */
>
> This statement is not true after 3a9b18b.
> "These checks are the same as we do in pg_terminate_backend."
>
> But the code is still correct, I assume... or not? In fact, we are
> killing autovacuum workers which are working with a given database
> (proc->roleId == 0), which is OK in that case. Are there any other
> cases when proc->roleId == 0 but we should not be able to kill such a
> process?
>

Good question. I am not aware of such cases but I wonder if we should
add a check similar to 3a9b18b [1] for the reason given in the commit
message. I have added Noah to see if he has any suggestions on this
matter.

[1] -
commit 3a9b18b3095366cd0c4305441d426d04572d88c1
Author: Noah Misch <noah(at)leadboat(dot)com>
Date: Mon Nov 6 06:14:13 2023 -0800

Ban role pg_signal_backend from more superuser backend types.

--
With Regards,
Amit Kapila.