On Wed, Oct 02, 2019 at 09:09:53PM -0700, Andres Freund wrote: > On 2019-10-03 11:03:39 +0900, Michael Paquier wrote: >> The idea here is to have a hook which can be triggered at the start of >> a process which can be externally triggered, which I guess is normal >> even for WAL senders not connected to a database. > > But what can you do in that situation? Without a database connection, > for example, you better not consider inserting anything into a table.
Adding extra custom logging information, or plug that information elsewhere than Postgres. I have use cases for that when it comes to store external telemetry data or audit things for events happening specifically in Postgres. > I have *SERIOUS* problems with performing additional writing > transactions in case of a FATAL. Something might have thrown that for a > reason, and just carrying on executing an arbitrary amount of code, this > might even involve triggers etc, imo seriously is not OK. Well, hook authors can do a lot of stupid things.. Anyway it looks that the end hook is out of scope as far as the discussion has gone based on the lack of facility, and that there is still interest for the start hook. I am just attaching a separate patch which adds only the start hook. The discussion could always respawn from that. This takes care of the HBA configuration when testing on Windows, and the module is still named "hooks" intentionally to avoid a future rename. -- Michael
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index e8d8e6f828..6d80cc2d64 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -171,6 +171,9 @@ static ProcSignalReason RecoveryConflictReason;
static MemoryContext row_description_context = NULL;
static StringInfoData row_description_buf;
+/* Hook for plugins to get control at start of session */
+session_start_hook_type session_start_hook = NULL;
+
/* ----------------------------------------------------------------
* decls for routines only used in this file
* ----------------------------------------------------------------
@@ -3968,6 +3971,9 @@ PostgresMain(int argc, char *argv[],
if (!IsUnderPostmaster)
PgStartTime = GetCurrentTimestamp();
+ if (session_start_hook)
+ (*session_start_hook) ();
+
/*
* POSTGRES main processing loop begins here
*
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index ec21f7e45c..f42935f019 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -30,6 +30,10 @@ extern PGDLLIMPORT const char *debug_query_string;
extern int max_stack_depth;
extern int PostAuthDelay;
+/* Hook for plugins to get control at start session */
+typedef void (*session_start_hook_type) (void);
+extern PGDLLIMPORT session_start_hook_type session_start_hook;
+
/* GUC-configurable parameters */
typedef enum
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index b2eaef3bff..abb3203903 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -21,6 +21,7 @@ SUBDIRS = \
test_predtest \
test_rbtree \
test_rls_hooks \
+ test_session_hooks \
test_shm_mq \
unsafe_tests \
worker_spi
diff --git a/src/test/modules/test_session_hooks/.gitignore b/src/test/modules/test_session_hooks/.gitignore
new file mode 100644
index 0000000000..5dcb3ff972
--- /dev/null
+++ b/src/test/modules/test_session_hooks/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/test_session_hooks/Makefile b/src/test/modules/test_session_hooks/Makefile
new file mode 100644
index 0000000000..a6fdb0434a
--- /dev/null
+++ b/src/test/modules/test_session_hooks/Makefile
@@ -0,0 +1,23 @@
+# src/test/modules/test_session_hooks/Makefile
+
+MODULE_big = test_session_hooks
+OBJS = test_session_hooks.o $(WIN32RES)
+PGFILEDESC = "test_session_hooks - tests for start session hook"
+
+REGRESS = test_session_hooks
+REGRESS_OPTS = --create-role=regress_sess_hook_usr1,regress_sess_hook_usr2 --temp-config=$(top_srcdir)/src/test/modules/test_session_hooks/session_hooks.conf
+# Disabled because these tests require extra configuration with
+# "shared_preload_libraries=test_session_hooks", which typical
+# installcheck users do not have (e.g. buildfarm clients).
+NO_INSTALLCHECK = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_session_hooks
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_session_hooks/README b/src/test/modules/test_session_hooks/README
new file mode 100644
index 0000000000..e945ffc45b
--- /dev/null
+++ b/src/test/modules/test_session_hooks/README
@@ -0,0 +1,10 @@
+test_session_hooks
+==================
+
+test_session_hooks is an example of how to use hook for session start.
+
+This module will insert into a pre-existing table called "session_hook_log"
+a log activity which happens at session start. It is possible to control
+which user information is logged when using the configuration parameter
+"test_session_hooks.username". If set, the hooks will log only information
+of the session user matching the parameter value.
diff --git a/src/test/modules/test_session_hooks/expected/test_session_hooks.out b/src/test/modules/test_session_hooks/expected/test_session_hooks.out
new file mode 100644
index 0000000000..0992b990ba
--- /dev/null
+++ b/src/test/modules/test_session_hooks/expected/test_session_hooks.out
@@ -0,0 +1,34 @@
+--
+-- Tests for start session hook
+--
+-- Only activity from role regress_sess_hook_usr2 is logged.
+ALTER ROLE regress_sess_hook_usr1 SUPERUSER LOGIN;
+ALTER ROLE regress_sess_hook_usr2 SUPERUSER LOGIN;
+\set prevdb :DBNAME
+\set prevusr :USER
+CREATE TABLE session_hook_log(id SERIAL, dbname TEXT, username TEXT, hook_at TEXT);
+SELECT * FROM session_hook_log ORDER BY id;
+ id | dbname | username | hook_at
+----+--------+----------+---------
+(0 rows)
+
+\c :prevdb regress_sess_hook_usr1
+SELECT * FROM session_hook_log ORDER BY id;
+ id | dbname | username | hook_at
+----+--------+----------+---------
+(0 rows)
+
+\c :prevdb regress_sess_hook_usr2
+SELECT * FROM session_hook_log ORDER BY id;
+ id | dbname | username | hook_at
+----+--------------------+------------------------+---------
+ 1 | contrib_regression | regress_sess_hook_usr2 | START
+(1 row)
+
+\c :prevdb :prevusr
+SELECT * FROM session_hook_log ORDER BY id;
+ id | dbname | username | hook_at
+----+--------------------+------------------------+---------
+ 1 | contrib_regression | regress_sess_hook_usr2 | START
+(1 row)
+
diff --git a/src/test/modules/test_session_hooks/session_hooks.conf b/src/test/modules/test_session_hooks/session_hooks.conf
new file mode 100644
index 0000000000..fc62b4adef
--- /dev/null
+++ b/src/test/modules/test_session_hooks/session_hooks.conf
@@ -0,0 +1,2 @@
+shared_preload_libraries = 'test_session_hooks'
+test_session_hooks.username = regress_sess_hook_usr2
diff --git a/src/test/modules/test_session_hooks/sql/test_session_hooks.sql b/src/test/modules/test_session_hooks/sql/test_session_hooks.sql
new file mode 100644
index 0000000000..01a285e9a5
--- /dev/null
+++ b/src/test/modules/test_session_hooks/sql/test_session_hooks.sql
@@ -0,0 +1,17 @@
+--
+-- Tests for start session hook
+--
+
+-- Only activity from role regress_sess_hook_usr2 is logged.
+ALTER ROLE regress_sess_hook_usr1 SUPERUSER LOGIN;
+ALTER ROLE regress_sess_hook_usr2 SUPERUSER LOGIN;
+\set prevdb :DBNAME
+\set prevusr :USER
+CREATE TABLE session_hook_log(id SERIAL, dbname TEXT, username TEXT, hook_at TEXT);
+SELECT * FROM session_hook_log ORDER BY id;
+\c :prevdb regress_sess_hook_usr1
+SELECT * FROM session_hook_log ORDER BY id;
+\c :prevdb regress_sess_hook_usr2
+SELECT * FROM session_hook_log ORDER BY id;
+\c :prevdb :prevusr
+SELECT * FROM session_hook_log ORDER BY id;
diff --git a/src/test/modules/test_session_hooks/test_session_hooks.c b/src/test/modules/test_session_hooks/test_session_hooks.c
new file mode 100644
index 0000000000..0e69ddf954
--- /dev/null
+++ b/src/test/modules/test_session_hooks/test_session_hooks.c
@@ -0,0 +1,134 @@
+/* -------------------------------------------------------------------------
+ *
+ * test_session_hooks.c
+ * Code for testing hook for session start.
+ *
+ * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ * src/test/modules/test_session_hooks/test_session_hooks.c
+ *
+ * -------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "access/parallel.h"
+#include "access/xact.h"
+#include "access/xlog.h"
+#include "commands/dbcommands.h"
+#include "executor/spi.h"
+#include "lib/stringinfo.h"
+#include "miscadmin.h"
+#include "tcop/tcopprot.h"
+#include "utils/snapmgr.h"
+#include "utils/builtins.h"
+
+PG_MODULE_MAGIC;
+
+/* Entry point of library loading/unloading */
+void _PG_init(void);
+void _PG_fini(void);
+
+/* GUC variables */
+static char *session_hook_username = "postgres";
+
+/* Previous hook on stack */
+static session_start_hook_type prev_session_start_hook = NULL;
+
+static void
+register_session_hook(const char *hook_at)
+{
+ const char *username;
+
+ StartTransactionCommand();
+ SPI_connect();
+ PushActiveSnapshot(GetTransactionSnapshot());
+
+ /* Check the current user validity */
+ username = GetUserNameFromId(GetUserId(), false);
+
+ /* Register log just for configured username */
+ if (strcmp(username, session_hook_username) == 0)
+ {
+ const char *dbname;
+ int ret;
+ StringInfoData buf;
+
+ dbname = get_database_name(MyDatabaseId);
+
+ initStringInfo(&buf);
+
+ appendStringInfo(&buf, "INSERT INTO session_hook_log (dbname, username, hook_at) ");
+ appendStringInfo(&buf, "VALUES (%s, %s, %s);",
+ quote_literal_cstr(dbname),
+ quote_literal_cstr(username),
+ quote_literal_cstr(hook_at));
+
+ ret = SPI_exec(buf.data, 0);
+ if (ret != SPI_OK_INSERT)
+ elog(ERROR, "SPI_execute failed: error code %d", ret);
+ }
+
+ SPI_finish();
+ PopActiveSnapshot();
+ CommitTransactionCommand();
+}
+
+/* sample session start hook function */
+static void
+sample_session_start_hook(void)
+{
+ if (prev_session_start_hook)
+ prev_session_start_hook();
+
+ /* nothing to do while in recovery */
+ if (RecoveryInProgress())
+ return;
+
+ /* consider only normal backends */
+ if (MyBackendId == InvalidBackendId)
+ return;
+
+ /* consider backends connected to a database */
+ if (!OidIsValid(MyDatabaseId))
+ return;
+
+ /* no parallel workers */
+ if (IsParallelWorker())
+ return;
+
+ register_session_hook("START");
+}
+
+/*
+ * Module load callback
+ */
+void
+_PG_init(void)
+{
+ /* Save previous hook */
+ prev_session_start_hook = session_start_hook;
+
+ /* Set new hook */
+ session_start_hook = sample_session_start_hook;
+
+ /* Load GUCs */
+ DefineCustomStringVariable("test_session_hooks.username",
+ "Username to register log on session start",
+ NULL,
+ &session_hook_username,
+ "postgres",
+ PGC_SIGHUP,
+ 0, NULL, NULL, NULL);
+}
+
+/*
+ * Module unload callback
+ */
+void
+_PG_fini(void)
+{
+ /* Uninstall hook */
+ session_start_hook = prev_session_start_hook;
+}
signature.asc
Description: PGP signature
