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

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
 	 *

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) ();
 }
 
 

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

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

src/test/modules/test_session_hooks/.gitignore

-# Generated subdirectories
-/log/
-/results/
-/tmp_check/

src/test/modules/test_session_hooks/Makefile

-# 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

src/test/modules/test_session_hooks/README

-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.

src/test/modules/test_session_hooks/expected/test_session_hooks.out

---
--- 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;

src/test/modules/test_session_hooks/session_hooks.conf

-shared_preload_libraries = 'test_session_hooks'
-test_session_hooks.username = regress_sess_hook_usr2

src/test/modules/test_session_hooks/sql/test_session_hooks.sql

---
--- 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;

src/test/modules/test_session_hooks/test_session_hooks.c

-/* -------------------------------------------------------------------------
- *
- * 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;
-}