Re: On login trigger: take three - Mailing list pgsql-hackers

From Konstantin Knizhnik
Subject Re: On login trigger: take three
Date
Msg-id b296d667-ba8d-5aad-4b75-e4d7286ebc1f@postgrespro.ru
Whole thread Raw
In response to Re: On login trigger: take three  (Pavel Stehule <pavel.stehule@gmail.com>)
Responses Re: On login trigger: take three
List pgsql-hackers


On 22.12.2020 21:19, Pavel Stehule wrote:


út 22. 12. 2020 v 12:42 odesílatel Konstantin Knizhnik <k.knizhnik@postgrespro.ru> napsal:


On 22.12.2020 12:25, Pavel Stehule wrote:

regress tests fails

     sysviews                     ... FAILED      112 ms
test event_trigger                ... FAILED (test process exited with exit code 2)      447 ms
test fast_default                 ... FAILED      392 ms
test stats                        ... FAILED      626 ms
============== shutting down postmaster               ==============


Sorry, fixed.

no problem

I had to fix doc

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6ff783273f..7aded1848f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1008,7 +1008,7 @@ include_dir 'conf.d'
         trigger when a client connects. This parameter is switched on by default.
         Errors in trigger code can prevent user to login to the system.
         I this case disabling this parameter in connection string can solve the problem:
-        <literal>psql "dbname=postgres options='-c enable_client_connection_trigger=false'".
+        <literal>psql "dbname=postgres options='-c enable_client_connection_trigger=false'"</literal>.
        </para>
       </listitem>
      </varlistentry>

I am thinking again about enable_client_connection_trigger, and although it can look useless (because error is ignored for superuser), it can be useful for some debugging and administration purposes. Probably we don't want to start the client_connection trigger from backup tools, maybe from some monitoring tools. Maybe the possibility to set this GUC can be dedicated to some special role (like pg_signal_backend).

+     <varlistentry id="guc-enable-client-connection-trigger" xreflabel="enable_client_connection_trigger">
+      <term><varname>enable_client_connection_trigger</varname> (<type>boolean</type>)
+      <indexterm>
+       <primary><varname>enable_client_connection_trigger</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Enables firing the <literal>client_connection</literal>
+        trigger when a client connects. This parameter is switched on by default.
+        Errors in trigger code can prevent user to login to the system.
+        I this case disabling this parameter in connection string can solve the problem:
+        <literal>psql "dbname=postgres options='-c enable_client_connection_trigger=false'"</literal>.
+       </para>
+      </listitem>
+     </varlistentry>

There should be note, so only superuser can change this value

There is should be tab-complete support

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 3a43c09bf6..08f00d8fc4 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2970,7 +2970,8 @@ psql_completion(const char *text, int start, int end)
        COMPLETE_WITH("ON");
    /* Complete CREATE EVENT TRIGGER <name> ON with event_type */
    else if (Matches("CREATE", "EVENT", "TRIGGER", MatchAny, "ON"))
-       COMPLETE_WITH("ddl_command_start", "ddl_command_end", "sql_drop");
+       COMPLETE_WITH("ddl_command_start", "ddl_command_end",
+                     "client_connection", "sql_drop");
 
    /*
     * Complete CREATE EVENT TRIGGER <name> ON <event_type>.  EXECUTE FUNCTION

Thank you.
I have applied all your fixes in on_connect_event_trigger-12.patch.

Concerning enable_client_connection_trigger GUC, I think that it is really useful: it is the fastest and simplest way to disable login triggers in case
of some problems with them (not only for superuser itself, but for all users). Yes, it can be also done using "ALTER EVENT TRIGGER DISABLE".
But assume that you have a lot of databases with different login policies enforced by on-login event triggers. And you want temporary disable them all, for example for testing purposes.
In this case GUC is most convenient way to do it.



Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company 
Attachment

pgsql-hackers by date:

Previous
From: "Tang, Haiying"
Date:
Subject: RE: [Patch] Optimize dropping of relation buffers using dlist
Next
From: Masahiko Sawada
Date:
Subject: Re: Commit fest manager for 2021-01