Hi all, Attached is a patch set to respawn the issue of $subject which has been discussed here: https://www.postgresql.org/message-id/20170720204733.40f2b7eb.nag...@sraoss.co.jp
The patch has been committed once as of cd8ce3a but it got shortly reverted after with 98d54bb because of buildfarm failures. The root of the buildfarm issues was that session hooks cannot be tested with a simple LOAD, hence we need to mark the test with NO_INSTALLCHECK. Unfortunately we lacked support for that in MSVC scripts, until I solved that with 431f1599 when refactoring PGXS makefile rules for regression tests. While on it, I have done a review over the patch, cleaning it up a bit and I found some issues, so the initial patch was not fully baked either: - previous hook calls were only called for normal backends, which was incorrect as we define the backend so as we apply no backend-related filtering for the hook. - The README could be also more talkative. - test_session_hooks--1.0.sql also got confused with the module name. And actually there is no need to have the SQL and control files just for a module loading a hook. So it is enough to use MODULE_big for this purpose. - The query generated needs to use quote_literal_cstr for the string values added to the query. - sample_session_start_hook and sample_session_end_hook missed a (void), causing a compiler warning on Windows. Attached is an updated patch set to reintroduce the hook, as there was a ask for it recently, fixing also the issue that we previously tried to deal with. I have tested the patch on Windows to make sure that the test gets correctly bypassed, so this time we should not have any buildfarm failures. I am adding that to next CF. Credits go of course to the initial authors and reviewers of this feature. Any opinions? -- 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/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 43b9f17f72..4037267be9 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -78,6 +78,8 @@ static bool ThereIsAtLeastOneRole(void);
static void process_startup_options(Port *port, bool am_superuser);
static void process_settings(Oid databaseid, Oid roleid);
+/* Hook for plugins to get control at end of session */
+session_end_hook_type session_end_hook = NULL;
/*** InitPostgres support ***/
@@ -1197,6 +1199,10 @@ ShutdownPostgres(int code, Datum arg)
* them explicitly.
*/
LockReleaseAll(USER_LOCKMETHOD, true);
+
+ /* Hook at session end */
+ if (session_end_hook)
+ (*session_end_hook) ();
}
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index ec21f7e45c..63581048b2 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -30,6 +30,13 @@ extern PGDLLIMPORT const char *debug_query_string;
extern int max_stack_depth;
extern int PostAuthDelay;
+/* Hook for plugins to get control at start and end of session */
+typedef void (*session_start_hook_type) (void);
+typedef void (*session_end_hook_type) (void);
+
+extern PGDLLIMPORT session_start_hook_type session_start_hook;
+extern PGDLLIMPORT session_end_hook_type session_end_hook;
+
/* GUC-configurable parameters */
typedef enum
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 60d6d7be1b..066b98e114 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -18,6 +18,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..f6fcdeaf20
--- /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 - Test session hooks with an extension"
+
+REGRESS = test_session_hooks
+REGRESS_OPTS = --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..bbbeaa65f0
--- /dev/null
+++ b/src/test/modules/test_session_hooks/README
@@ -0,0 +1,8 @@
+test_session_hooks is an example of how to use session start and end
+hooks.
+
+This module will insert into a pre-existing table called "session_hook_log"
+a log activity which happens at session start and end. 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..be1b94953c
--- /dev/null
+++ b/src/test/modules/test_session_hooks/expected/test_session_hooks.out
@@ -0,0 +1,31 @@
+CREATE ROLE regress_sess_hook_usr1 SUPERUSER LOGIN;
+CREATE 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
+ 2 | contrib_regression | regress_sess_hook_usr2 | END
+(2 rows)
+
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..5e0864753d
--- /dev/null
+++ b/src/test/modules/test_session_hooks/sql/test_session_hooks.sql
@@ -0,0 +1,12 @@
+CREATE ROLE regress_sess_hook_usr1 SUPERUSER LOGIN;
+CREATE 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..a6fba2d2db
--- /dev/null
+++ b/src/test/modules/test_session_hooks/test_session_hooks.c
@@ -0,0 +1,136 @@
+/* -------------------------------------------------------------------------
+ *
+ * test_session_hooks.c
+ * Code for testing SESSION hooks.
+ *
+ * Copyright (c) 2010-2017, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/test/modules/test_session_hooks/test_session_hooks.c
+ *
+ * -------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "access/xact.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";
+
+/* Original Hook */
+static session_start_hook_type prev_session_start_hook = NULL;
+static session_end_hook_type prev_session_end_hook = NULL;
+
+static void
+register_session_hook(const char *hook_at)
+{
+ const char *username;
+
+ StartTransactionCommand();
+ SPI_connect();
+ PushActiveSnapshot(GetTransactionSnapshot());
+
+ username = GetUserNameFromId(GetUserId(), false);
+
+ /* Register log just for configured username */
+ if (!strcmp(username, session_hook_username))
+ {
+ 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();
+
+ /* just consider normal backends */
+ if (MyBackendId == InvalidBackendId)
+ return;
+
+ (void) register_session_hook("START");
+}
+
+/* sample session end hook function */
+static void
+sample_session_end_hook(void)
+{
+ if (prev_session_end_hook)
+ prev_session_end_hook();
+
+ /* just consider normal backends */
+ if (MyBackendId == InvalidBackendId)
+ return;
+
+ (void) register_session_hook("END");
+}
+
+/*
+ * Module Load Callback
+ */
+void
+_PG_init(void)
+{
+ /* Save Hooks for Unload */
+ prev_session_start_hook = session_start_hook;
+ prev_session_end_hook = session_end_hook;
+
+ /* Set New Hooks */
+ session_start_hook = sample_session_start_hook;
+ session_end_hook = sample_session_end_hook;
+
+ /* Load GUCs */
+ DefineCustomStringVariable("test_session_hooks.username",
+ "Username to register log on session start or end",
+ NULL,
+ &session_hook_username,
+ "postgres",
+ PGC_SIGHUP,
+ 0, NULL, NULL, NULL);
+}
+
+/*
+ * Module Unload Callback
+ */
+void
+_PG_fini(void)
+{
+ /* Uninstall Hooks */
+ session_start_hook = prev_session_start_hook;
+ session_end_hook = prev_session_end_hook;
+}
signature.asc
Description: PGP signature
