On Tue, Nov 7, 2017 at 1:15 AM, Michael Paquier <[email protected]>
wrote:
>
> On Sun, Nov 5, 2017 at 3:14 AM, Fabrízio de Royes Mello
> <[email protected]> wrote:
> > On Sat, Nov 4, 2017 at 1:23 AM, Michael Paquier <
[email protected]>
> > wrote:
> >> On Fri, Nov 3, 2017 at 1:55 PM, Fabrízio de Royes Mello
> >> <[email protected]> wrote:
> >> >> Passing the database name and user name does not look much useful to
> >> >> me. You can have access to this data already with CurrentUserId and
> >> >> MyDatabaseId.
> >> >
> >> > This way we don't need to convert oid to names inside hook code.
> >>
> >> Well, arguments of hooks are useful if they are used. Now if I look at
> >> the two examples mainly proposed in this thread, be it in your set of
> >> patches or the possibility to do some SQL transaction, I see nothing
> >> using them. So I'd vote for keeping an interface minimal.
> >>
> >
> > Maybe the attached patch with improved test module can illustrate
better the
> > feature.
>
> I was going to to hack something like that. That's interesting for the
> use case Robert has mentioned.
>
> Well, in the case of the end session hook, those variables are passed
> to the hook by being directly taken from the context from MyProcPort:
> + (*session_end_hook) (MyProcPort->database_name,
> MyProcPort->user_name);
> In the case of the start hook, those are directly taken from the
> command outer caller, but similarly MyProcPort is already set, so
> those are strings available (your patch does so in the end session
> hook)... Variables in hooks are useful if those are not available
> within the memory context, and refer to a specific state where the
> hook is called. For example, take the password hook, this uses the
> user name and the password because those values are not available
> within the session context. The same stands for other hooks as well.
> Keeping the interface minimal helps in readability and maintenance.
> See for the attached example that can be applied on top of 0003, which
> makes use of the session context, the set regression tests does not
> pass but this shows how I think those hooks had better be shaped.
>
Makes sense... fixed.
> + (*session_end_hook) (MyProcPort->database_name,
MyProcPort->user_name);
> +
> + /* Make don't leave any active transactions and/or locks behind */
> + AbortOutOfAnyTransaction();
> + LockReleaseAll(USER_LOCKMETHOD, true);
> Let's leave this work to people actually implementing the hook contents.
>
Fixed.
Attached new version. I unify all three patches in just one because this is
a small patch and it's most part is test code.
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2c7260e..52a9641 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -169,6 +169,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
* ----------------------------------------------------------------
@@ -3857,6 +3860,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 20f1d27..ffaf51f 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -76,6 +76,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 ***/
@@ -1154,6 +1156,10 @@ ShutdownPostgres(int code, Datum arg)
* them explicitly.
*/
LockReleaseAll(USER_LOCKMETHOD, true);
+
+ /* Hook just normal backends */
+ if (session_end_hook && MyBackendId != InvalidBackendId)
+ (*session_end_hook) ();
}
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index f8c535c..9f05bfb 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -35,6 +35,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 b7ed0af..a3c8c1e 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -11,6 +11,7 @@ SUBDIRS = \
snapshot_too_old \
test_ddl_deparse \
test_extensions \
+ test_session_hooks \
test_parser \
test_pg_dump \
test_rbtree \
diff --git a/src/test/modules/test_session_hooks/.gitignore b/src/test/modules/test_session_hooks/.gitignore
new file mode 100644
index 0000000..5dcb3ff
--- /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 0000000..c5c3860
--- /dev/null
+++ b/src/test/modules/test_session_hooks/Makefile
@@ -0,0 +1,21 @@
+# src/test/modules/test_session_hooks/Makefile
+
+MODULES = test_session_hooks
+PGFILEDESC = "test_session_hooks - Test session hooks with an extension"
+
+EXTENSION = test_session_hooks
+DATA = test_session_hooks--1.0.sql
+
+REGRESS = test_session_hooks
+REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/test_session_hooks/session_hooks.conf
+
+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 0000000..9cb4202
--- /dev/null
+++ b/src/test/modules/test_session_hooks/README
@@ -0,0 +1,2 @@
+test_session_hooks is an example of how to use session start and end
+hooks.
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 0000000..55b48ff
--- /dev/null
+++ b/src/test/modules/test_session_hooks/expected/test_session_hooks.out
@@ -0,0 +1,40 @@
+CREATE ROLE session_hook_usr1 LOGIN;
+CREATE ROLE session_hook_usr2 LOGIN;
+CREATE DATABASE session_hook_db;
+\set prevdb :DBNAME
+\set prevusr :USER
+\c session_hook_db
+CREATE TABLE session_hook_log(id SERIAL, dbname TEXT, username TEXT, hook_at TEXT);
+GRANT INSERT ON session_hook_log TO PUBLIC;
+GRANT USAGE ON SEQUENCE session_hook_log_id_seq TO PUBLIC;
+\c :prevdb :prevusr
+ALTER SYSTEM SET test_session_hooks.start_enabled TO true;
+ALTER SYSTEM SET test_session_hooks.end_enabled TO true;
+SELECT pg_reload_conf();
+ pg_reload_conf
+----------------
+ t
+(1 row)
+
+\c session_hook_db session_hook_usr1
+\c :prevdb :prevusr
+\c session_hook_db session_hook_usr2
+\c :prevdb :prevusr
+ALTER SYSTEM SET test_session_hooks.start_enabled TO false;
+ALTER SYSTEM SET test_session_hooks.end_enabled TO false;
+SELECT pg_reload_conf();
+ pg_reload_conf
+----------------
+ t
+(1 row)
+
+\c session_hook_db
+SELECT * FROM session_hook_log ORDER BY id;
+ id | dbname | username | hook_at
+----+-----------------+-------------------+---------
+ 1 | session_hook_db | session_hook_usr1 | START
+ 2 | session_hook_db | session_hook_usr1 | END
+ 3 | session_hook_db | session_hook_usr2 | START
+ 4 | session_hook_db | session_hook_usr2 | END
+(4 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 0000000..a66e60e
--- /dev/null
+++ b/src/test/modules/test_session_hooks/session_hooks.conf
@@ -0,0 +1 @@
+shared_preload_libraries = 'test_session_hooks'
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 0000000..71b9f8e
--- /dev/null
+++ b/src/test/modules/test_session_hooks/sql/test_session_hooks.sql
@@ -0,0 +1,22 @@
+CREATE ROLE session_hook_usr1 LOGIN;
+CREATE ROLE session_hook_usr2 LOGIN;
+CREATE DATABASE session_hook_db;
+\set prevdb :DBNAME
+\set prevusr :USER
+\c session_hook_db
+CREATE TABLE session_hook_log(id SERIAL, dbname TEXT, username TEXT, hook_at TEXT);
+GRANT INSERT ON session_hook_log TO PUBLIC;
+GRANT USAGE ON SEQUENCE session_hook_log_id_seq TO PUBLIC;
+\c :prevdb :prevusr
+ALTER SYSTEM SET test_session_hooks.start_enabled TO true;
+ALTER SYSTEM SET test_session_hooks.end_enabled TO true;
+SELECT pg_reload_conf();
+\c session_hook_db session_hook_usr1
+\c :prevdb :prevusr
+\c session_hook_db session_hook_usr2
+\c :prevdb :prevusr
+ALTER SYSTEM SET test_session_hooks.start_enabled TO false;
+ALTER SYSTEM SET test_session_hooks.end_enabled TO false;
+SELECT pg_reload_conf();
+\c session_hook_db
+SELECT * FROM session_hook_log ORDER BY id;
diff --git a/src/test/modules/test_session_hooks/test_session_hooks--1.0.sql b/src/test/modules/test_session_hooks/test_session_hooks--1.0.sql
new file mode 100644
index 0000000..16bcee9
--- /dev/null
+++ b/src/test/modules/test_session_hooks/test_session_hooks--1.0.sql
@@ -0,0 +1,4 @@
+/* src/test/modules/test_hook_session/test_hook_session--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_hook_session" to load this file. \quit
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 0000000..ee1d9b6
--- /dev/null
+++ b/src/test/modules/test_session_hooks/test_session_hooks.c
@@ -0,0 +1,134 @@
+/* -------------------------------------------------------------------------
+ *
+ * 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 bool session_start_hook_enabled = false;
+static bool session_end_hook_enabled = false;
+
+/* 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)
+{
+ int ret;
+ StringInfoData buf;
+ const char *dbname;
+ const char *username;
+
+ StartTransactionCommand();
+ SPI_connect();
+ PushActiveSnapshot(GetTransactionSnapshot());
+
+ dbname = get_database_name(MyDatabaseId);
+ username = GetUserNameFromId(GetUserId(), false);
+
+ if (!strcmp(dbname, "session_hook_db"))
+ {
+ initStringInfo(&buf);
+
+ appendStringInfo(&buf, "INSERT INTO session_hook_log (dbname, username, hook_at) ");
+ appendStringInfo(&buf, "VALUES ('%s', '%s', '%s');",
+ dbname, username, hook_at);
+
+ ret = SPI_exec(buf.data, 0);
+ if (ret != SPI_OK_INSERT)
+ elog(FATAL, "SPI_execute failed: error code %d", ret);
+ }
+
+ SPI_finish();
+ PopActiveSnapshot();
+ CommitTransactionCommand();
+}
+
+/* sample session start hook function */
+static void
+sample_session_start_hook()
+{
+ if (prev_session_start_hook)
+ prev_session_start_hook();
+
+ if (session_start_hook_enabled)
+ (void) register_session_hook("START");
+}
+
+/* sample sessoin end hook function */
+static void
+sample_session_end_hook(const char *dbname, const char *username)
+{
+ if (prev_session_end_hook)
+ prev_session_end_hook();
+
+ if (session_end_hook_enabled)
+ (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 */
+ DefineCustomBoolVariable("test_session_hooks.start_enabled",
+ "Enable/Disable session start hook",
+ NULL,
+ &session_start_hook_enabled,
+ false,
+ PGC_SIGHUP,
+ 0, NULL, NULL, NULL);
+
+ DefineCustomBoolVariable("test_session_hooks.end_enabled",
+ "Enable/Disable session end hook",
+ NULL,
+ &session_end_hook_enabled,
+ false,
+ 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;
+}
diff --git a/src/test/modules/test_session_hooks/test_session_hooks.control b/src/test/modules/test_session_hooks/test_session_hooks.control
new file mode 100644
index 0000000..7d7ef9f
--- /dev/null
+++ b/src/test/modules/test_session_hooks/test_session_hooks.control
@@ -0,0 +1,3 @@
+comment = 'Test start/end hook session with an extension'
+default_version = '1.0'
+relocatable = true
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
