On Fri, Sep 22, 2023 at 05:29:01PM +0200, Daniel Gustafsson wrote: > Since the main driver for this is to reduce the usage/need for single-user > mode > I also reworded the patch slightly. Instead of phrasing this as an > alternative > to single-user mode, I reversed it such that single-user mode is an > alternative > to this GUC.
Okay. + be disabled by setting it to <literal>false</literal>. Setting the value + to <literal>true</literal> will not disable any event triggers, this This uses a double negation. Perhaps just "Setting this value to true allows all event triggers to run." 003_check_guc.pl has detected a failure because event_triggers is missing in postgresql.conf.sample while it is not marked with GUC_NOT_IN_SAMPLE anymore. Keeping the docs consistent with the sample file, I would suggest the attached on top of your v9. -- Michael
From 2ad4989a71c40b7d6de8fcb095b057377cd05bf5 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson <dgustafs...@postgresql.org> Date: Fri, 22 Sep 2023 16:39:07 +0200 Subject: [PATCH v9 1/2] Add GUC for temporarily disabling event triggers In order to troubleshoot misbehaving or buggy event triggers, the documented advice is to enter single-user mode. In an attempt to reduce the number of situations where single-user mode is required (or even recommended) for non-extraordinary maintenance, this GUC allows to temporarily suspend event triggers. This was originally extracted from a larger patchset which aimed at supporting event triggers on login events. Reviewed-by: Ted Yu <yuzhih...@gmail.com> Reviewed-by: Mikhail Gribkov <youzh...@gmail.com> Reviewed-by: Justin Pryzby <pry...@telsasoft.com> Reviewed-by: Michael Paquier <mich...@paquier.xyz Reviewed-by: Robert Haas <robertmh...@gmail.com> Discussion: https://postgr.es/m/9140106e-f9bf-4d85-8fc8-f2d3c094a...@yesql.se --- src/include/commands/event_trigger.h | 2 ++ src/backend/commands/event_trigger.c | 20 +++++++++++------ src/backend/utils/misc/guc_tables.c | 11 ++++++++++ src/test/regress/expected/event_trigger.out | 22 +++++++++++++++++++ src/test/regress/sql/event_trigger.sql | 24 +++++++++++++++++++++ doc/src/sgml/config.sgml | 19 ++++++++++++++++ doc/src/sgml/ref/create_event_trigger.sgml | 9 +++++--- 7 files changed, 97 insertions(+), 10 deletions(-) diff --git a/src/include/commands/event_trigger.h b/src/include/commands/event_trigger.h index 5ed6ece555..1c925dbf25 100644 --- a/src/include/commands/event_trigger.h +++ b/src/include/commands/event_trigger.h @@ -29,6 +29,8 @@ typedef struct EventTriggerData CommandTag tag; } EventTriggerData; +extern PGDLLIMPORT bool event_triggers; + #define AT_REWRITE_ALTER_PERSISTENCE 0x01 #define AT_REWRITE_DEFAULT_VAL 0x02 #define AT_REWRITE_COLUMN_REWRITE 0x04 diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index d4b00d1a82..bd812e42d9 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -72,6 +72,9 @@ typedef struct EventTriggerQueryState static EventTriggerQueryState *currentEventTriggerState = NULL; +/* GUC parameter */ +bool event_triggers = true; + /* Support for dropped objects */ typedef struct SQLDropObject { @@ -657,8 +660,11 @@ EventTriggerDDLCommandStart(Node *parsetree) * wherein event triggers are disabled. (Or we could implement * heapscan-and-sort logic for that case, but having disaster recovery * scenarios depend on code that's otherwise untested isn't appetizing.) + * + * Additionally, event triggers can be disabled with a superuser-only GUC + * to make fixing database easier as per 1 above. */ - if (!IsUnderPostmaster) + if (!IsUnderPostmaster || !event_triggers) return; runlist = EventTriggerCommonSetup(parsetree, @@ -692,9 +698,9 @@ EventTriggerDDLCommandEnd(Node *parsetree) /* * See EventTriggerDDLCommandStart for a discussion about why event - * triggers are disabled in single user mode. + * triggers are disabled in single user mode or via GUC. */ - if (!IsUnderPostmaster) + if (!IsUnderPostmaster || !event_triggers) return; /* @@ -740,9 +746,9 @@ EventTriggerSQLDrop(Node *parsetree) /* * See EventTriggerDDLCommandStart for a discussion about why event - * triggers are disabled in single user mode. + * triggers are disabled in single user mode or via a GUC. */ - if (!IsUnderPostmaster) + if (!IsUnderPostmaster || !event_triggers) return; /* @@ -811,9 +817,9 @@ EventTriggerTableRewrite(Node *parsetree, Oid tableOid, int reason) /* * See EventTriggerDDLCommandStart for a discussion about why event - * triggers are disabled in single user mode. + * triggers are disabled in single user mode or via a GUC. */ - if (!IsUnderPostmaster) + if (!IsUnderPostmaster || !event_triggers) return; /* diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index bdb26e2b77..16ec6c5ef0 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -37,6 +37,7 @@ #include "catalog/namespace.h" #include "catalog/storage.h" #include "commands/async.h" +#include "commands/event_trigger.h" #include "commands/tablespace.h" #include "commands/trigger.h" #include "commands/user.h" @@ -2000,6 +2001,16 @@ struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, + { + {"event_triggers", PGC_SUSET, CLIENT_CONN_STATEMENT, + gettext_noop("Enables event triggers."), + gettext_noop("When enabled, event triggers will fire for all applicable statements."), + }, + &event_triggers, + true, + NULL, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out index 2c8a6b2212..0b87a42d0a 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -616,3 +616,25 @@ SELECT DROP EVENT TRIGGER start_rls_command; DROP EVENT TRIGGER end_rls_command; DROP EVENT TRIGGER sql_drop_command; +-- Check the GUC for disabling event triggers +CREATE FUNCTION test_event_trigger_guc() RETURNS event_trigger +LANGUAGE plpgsql AS $$ +DECLARE + obj record; +BEGIN + FOR obj IN SELECT * FROM pg_event_trigger_dropped_objects() + LOOP + RAISE NOTICE '% dropped %', tg_tag, obj.object_type; + END LOOP; +END; +$$; +CREATE EVENT TRIGGER test_event_trigger_guc + ON sql_drop + WHEN TAG IN ('DROP POLICY') EXECUTE FUNCTION test_event_trigger_guc(); +SET event_triggers = 'on'; +CREATE POLICY pguc ON event_trigger_test USING (FALSE); +DROP POLICY pguc ON event_trigger_test; +NOTICE: DROP POLICY dropped policy +CREATE POLICY pguc ON event_trigger_test USING (FALSE); +SET event_triggers = 'off'; +DROP POLICY pguc ON event_trigger_test; diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql index 1aeaddbe71..6f0933b9e8 100644 --- a/src/test/regress/sql/event_trigger.sql +++ b/src/test/regress/sql/event_trigger.sql @@ -471,3 +471,27 @@ SELECT DROP EVENT TRIGGER start_rls_command; DROP EVENT TRIGGER end_rls_command; DROP EVENT TRIGGER sql_drop_command; + +-- Check the GUC for disabling event triggers +CREATE FUNCTION test_event_trigger_guc() RETURNS event_trigger +LANGUAGE plpgsql AS $$ +DECLARE + obj record; +BEGIN + FOR obj IN SELECT * FROM pg_event_trigger_dropped_objects() + LOOP + RAISE NOTICE '% dropped %', tg_tag, obj.object_type; + END LOOP; +END; +$$; +CREATE EVENT TRIGGER test_event_trigger_guc + ON sql_drop + WHEN TAG IN ('DROP POLICY') EXECUTE FUNCTION test_event_trigger_guc(); + +SET event_triggers = 'on'; +CREATE POLICY pguc ON event_trigger_test USING (FALSE); +DROP POLICY pguc ON event_trigger_test; + +CREATE POLICY pguc ON event_trigger_test USING (FALSE); +SET event_triggers = 'off'; +DROP POLICY pguc ON event_trigger_test; diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 6bc1b215db..2bb3b35cf5 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -9425,6 +9425,25 @@ SET XML OPTION { DOCUMENT | CONTENT }; </listitem> </varlistentry> + <varlistentry id="guc-event-triggers" xreflabel="event_triggers"> + <term><varname>event_triggers</varname> (<type>boolean</type>) + <indexterm> + <primary><varname>event_triggers</varname></primary> + <secondary>configuration parameter</secondary> + </indexterm> + </term> + <listitem> + <para> + Allow temporarily disabling execution of event triggers in order to + troubleshoot and repair faulty event triggers. All event triggers will + be disabled by setting it to <literal>false</literal>. Setting the value + to <literal>true</literal> will not disable any event triggers, this + is the default value. Only superusers and users with the appropriate + <literal>SET</literal> privilege can change this setting. + </para> + </listitem> + </varlistentry> + </variablelist> </sect2> <sect2 id="runtime-config-client-format"> diff --git a/doc/src/sgml/ref/create_event_trigger.sgml b/doc/src/sgml/ref/create_event_trigger.sgml index 22c8119198..ef12cfa20d 100644 --- a/doc/src/sgml/ref/create_event_trigger.sgml +++ b/doc/src/sgml/ref/create_event_trigger.sgml @@ -121,9 +121,12 @@ CREATE EVENT TRIGGER <replaceable class="parameter">name</replaceable> <para> Event triggers are disabled in single-user mode (see <xref - linkend="app-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. + linkend="app-postgres"/>) as well as when + <xref linkend="guc-event-triggers"/> is set to <literal>false</literal>. + If an erroneous event trigger disables the database so much that you can't + even drop the trigger, restart with <xref linkend="guc-event-triggers"/> + set to <literal>false</literal> to temporarily disable event triggers, or + in single-user mode, and you'll be able to do that. </para> </refsect1> -- 2.40.1
From bd5ce7bbfc2b57f17ae647390e2de461b19e710a Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Mon, 25 Sep 2023 08:18:34 +0900 Subject: [PATCH v9 2/2] Fix failure in TAP tests. --- src/backend/utils/misc/postgresql.conf.sample | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 6bb39d39ba..d08d55c3fe 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -706,6 +706,7 @@ #xmloption = 'content' #gin_pending_list_limit = 4MB #createrole_self_grant = '' # set and/or inherit +#event_triggers = on # - Locale and Formatting - -- 2.40.1
signature.asc
Description: PGP signature
