Horton

Re: On login trigger: take three

Daniel Gustafsson <daniel@yesql.se>

View thread View on postgr.es
From: Daniel Gustafsson <daniel@yesql.se>
To: Andres Freund <andres@anarazel.de>
Cc: Greg Nancarrow <gregn4422@gmail.com>, Ivan Panchenko <wao@mail.ru>, Teodor Sigaev <teodor@sigaev.ru>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, vignesh C <vignesh21@gmail.com>, Pavel Stehule <pavel.stehule@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>
Date: 2022-03-13T22:31:03Z
Lists: pgsql-hackers 
> On 12 Mar 2022, at 03:46, Andres Freund <andres@anarazel.de> wrote:

>> +   <para>
>> +     The <literal>login</literal> event occurs when a user logs in to the
>> +     system.
>> +     Any bugs in a trigger procedure for this event may prevent successful
>> +     login to the system. Such bugs may be fixed after first restarting the
>> +     system in single-user mode (as event triggers are disabled in this mode).
>> +     See the <xref linkend="app-postgres"/> reference page for details about
>> +     using single-user mode.
>> +   </para>
> 
> I'm strongly against adding any new dependencies on single user mode.
> 
> A saner approach might be a superuser-only GUC that can be set as part of the
> connection data (e.g. PGOPTIONS='-c ignore_login_event_trigger=true').

This, and similar approaches, has been discussed in this thread.  I'm not a fan
of holes punched with GUC's like this, but if you plan on removing single-user
mode (as I recall seeing in an initdb thread) altogether then that kills the
discussion. So.

Since we already recommend single-user mode to handle broken event triggers, we
should IMO do something to cover all of them rather than risk ending up with
one disabling GUC per each event type.  Right now we have this on the CREATE
EVENT TRIGGER reference page:

    "Event triggers are disabled in single-user mode (see postgres).  If an
    erroneous event trigger disables the database so much that you can't even
    drop the trigger, restart in single-user mode and you'll be able to do
    that."

Something like a '-c ignore_event_trigger=<event|all>' GUC perhaps? 

--
Daniel Gustafsson		https://vmware.com/

Commits

  1. Fix some typos in event trigger docs

  2. Use heap_inplace_update() to unset pg_database.dathasloginevt

  3. Remove the flaky check in event_trigger_login regression test

  4. Fix instable 006_login_trigger.pl test

  5. Add support event triggers on authenticated login

  6. Add GUC for temporarily disabling event triggers

  7. Fix typo in reference to __FreeBSD__.

  8. Restore robustness of TAP tests that wait for postmaster restart.

  9. Restore the portal-level snapshot after procedure COMMIT/ROLLBACK.