Revert hooks for session start and end, take two

The location of the session end hook has been chosen so as it is
possible to allow modules to do their own transactions, however any
trying to any any subsystem which went through before_shmem_exit()
would cause issues, limiting the pluggability of the hook.

Per discussion with Tom Lane and Andres Freund.

Discussion: https://postgr.es/m/18722.1569906636@sss.pgh.pa.us

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 6d80cc2d64da5d396639766a75e6cf0af28396b0..e8d8e6f828516a6307f405538008f889059e03aa 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -171,9 +171,6 @@ 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
  * ----------------------------------------------------------------
@@ -3971,9 +3968,6 @@ 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 151703a43711956d6351de64bf35a6695dfaa7e7..29c5ec7b58bfbbd166afe12be6b42167b134c7a9 100644 (file)
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -78,8 +78,6 @@ 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,10 +1195,6 @@ 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 63581048b2cc593cc663f30d0d84add890985b47..ec21f7e45c5715196d2b57d698cededd26f4fc3f 100644 (file)
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -30,13 +30,6 @@ 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 abb3203903b001ac2802510159bc5540f9cea8de..b2eaef3bff5b945008a87499c0ae3db18b0356d5 100644 (file)
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -21,7 +21,6 @@ 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
deleted file mode 100644 (file)
index 5dcb3ff..0000000
--- a/src/test/modules/test_session_hooks/.gitignore
+++ /dev/null
@@ -1,4 +0,0 @@
-# Generated subdirectories
-/log/
-/results/
-/tmp_check/

diff --git a/src/test/modules/test_session_hooks/Makefile b/src/test/modules/test_session_hooks/Makefile
deleted file mode 100644 (file)
index e8ba699..0000000
--- a/src/test/modules/test_session_hooks/Makefile
+++ /dev/null
@@ -1,23 +0,0 @@
-# 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 and end session hooks"
-
-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
deleted file mode 100644 (file)
index 0fe3a98..0000000
--- a/src/test/modules/test_session_hooks/README
+++ /dev/null
@@ -1,11 +0,0 @@
-test_session_hooks
-==================
-
-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
deleted file mode 100644 (file)
index 120dcc9..0000000
--- a/src/test/modules/test_session_hooks/expected/test_session_hooks.out
+++ /dev/null
@@ -1,37 +0,0 @@
---
--- Tests for start and end session hooks
---
--- Only activity from role regress_sess_hook_usr2 is logged.
-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)
-
-DROP ROLE regress_sess_hook_usr1;
-DROP ROLE regress_sess_hook_usr2;

diff --git a/src/test/modules/test_session_hooks/session_hooks.conf b/src/test/modules/test_session_hooks/session_hooks.conf
deleted file mode 100644 (file)
index fc62b4a..0000000
--- a/src/test/modules/test_session_hooks/session_hooks.conf
+++ /dev/null
@@ -1,2 +0,0 @@
-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
deleted file mode 100644 (file)
index 1cc06f6..0000000
--- a/src/test/modules/test_session_hooks/sql/test_session_hooks.sql
+++ /dev/null
@@ -1,19 +0,0 @@
---
--- Tests for start and end session hooks
---
-
--- Only activity from role regress_sess_hook_usr2 is logged.
-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;
-DROP ROLE regress_sess_hook_usr1;
-DROP ROLE regress_sess_hook_usr2;

diff --git a/src/test/modules/test_session_hooks/test_session_hooks.c b/src/test/modules/test_session_hooks/test_session_hooks.c
deleted file mode 100644 (file)
index a90c373..0000000
--- a/src/test/modules/test_session_hooks/test_session_hooks.c
+++ /dev/null
@@ -1,155 +0,0 @@
-/* -------------------------------------------------------------------------
- *
- * test_session_hooks.c
- *             Code for testing start and end session hooks.
- *
- * 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 "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 hooks on stack */
-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());
-
-       /* 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();
-
-       /* 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");
-}
-
-/* sample session end hook function */
-static void
-sample_session_end_hook(void)
-{
-       if (prev_session_end_hook)
-               prev_session_end_hook();
-
-       /* 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("END");
-}
-
-/*
- * Module load callback
- */
-void
-_PG_init(void)
-{
-       /* Save previous hooks */
-       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;
-}