On Thu, Nov 4, 2021 at 7:43 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> On 3 Nov 2021, at 17:15, Ivan Panchenko <wao@mail.ru> wrote: > Среда, 29 сентября 2021, 15:14 +03:00 от Teodor Sigaev <teodor@sigaev.ru <x-msg://33/compose?To=teodor@sigaev.ru>>: > 2 For logging purpose failing of trigger will cause impossibility to login, it > could be workarounded by catching error in trigger function, but it's not so > obvious for DBA. > If the trigger contains an error, nobody can login. The database is bricked. > Previous variant of the patch proposed to fix this with going to single-user mode. > For faster recovery I propose to have also a GUC variable to turn on/off the login triggers. > It should be 'on' by default.
As voiced earlier, I disagree with this and I dislike the idea of punching a hole for circumventing infrastructure intended for auditing.
Use-cases for a login-trigger commonly involve audit trail logging, session initialization etc. If the login trigger bricks the production database to the extent that it needs to be restarted with the magic GUC, then it's highly likely that you *don't* want regular connections to the database for the duration of this. Any such connection won't be subject to what the trigger does which seem counter to having the trigger in the first place. This means that it's likely that the superuser fixing it will have to disable logins for everyone else while fixing, and it quicly becomes messy.
With that in mind, I think single-user mode actually *helps* the users in this case, and we avoid a hole punched which in worst case can be a vector for an attack.
Maybe I'm overly paranoid, but adding a backdoor of sorts for a situation which really shouldn't happen doesn't seem like a good idea.
+1
The arguments given are pretty convincing IMHO, and I agree that the additions made in the v20 patch are not a good idea, and are not needed.