From 98d54bb7790d5fb0a77173d5e5e3c655901b472c Mon Sep 17 00:00:00 2001 From: Andrew Dunstan Date: Thu, 16 Nov 2017 11:35:02 -0500 Subject: [PATCH] Back out the session_start and session_end hooks feature. It's become apparent during testing that there are problems with at least the testing regime. I don't think we should have it without a working test regime, and the difficulties might indicate implementation problems anyway, so I'm backing out the whole thing until that's sorted out. This reverts commits 7459484 9989f92 cd8ce3a --- src/backend/tcop/postgres.c | 6 - src/backend/utils/init/postinit.c | 6 - src/include/tcop/tcopprot.h | 7 - src/test/modules/Makefile | 1 - .../modules/test_session_hooks/.gitignore | 4 - src/test/modules/test_session_hooks/Makefile | 25 ---- src/test/modules/test_session_hooks/README | 2 - .../expected/test_session_hooks.out | 31 ---- .../test_session_hooks/session_hooks.conf | 2 - .../sql/test_session_hooks.sql | 12 -- .../test_session_hooks--1.0.sql | 4 - .../test_session_hooks/test_session_hooks.c | 134 ------------------ .../test_session_hooks.control | 3 - src/tools/msvc/vcregress.pl | 2 - 14 files changed, 239 deletions(-) delete mode 100644 src/test/modules/test_session_hooks/.gitignore delete mode 100644 src/test/modules/test_session_hooks/Makefile delete mode 100644 src/test/modules/test_session_hooks/README delete mode 100644 src/test/modules/test_session_hooks/expected/test_session_hooks.out delete mode 100644 src/test/modules/test_session_hooks/session_hooks.conf delete mode 100644 src/test/modules/test_session_hooks/sql/test_session_hooks.sql delete mode 100644 src/test/modules/test_session_hooks/test_session_hooks--1.0.sql delete mode 100644 src/test/modules/test_session_hooks/test_session_hooks.c delete mode 100644 src/test/modules/test_session_hooks/test_session_hooks.control diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index d3156ad49ef..05c5c194ec6 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -169,9 +169,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 * ---------------------------------------------------------------- @@ -3860,9 +3857,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 16ec376b221..20f1d279e9c 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -76,8 +76,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 ***/ @@ -1156,10 +1154,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 9f05bfb4ab5..f8c535c91e3 100644 --- a/src/include/tcop/tcopprot.h +++ b/src/include/tcop/tcopprot.h @@ -35,13 +35,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 7246552d386..b7ed0af021f 100644 --- a/src/test/modules/Makefile +++ b/src/test/modules/Makefile @@ -15,7 +15,6 @@ SUBDIRS = \ test_pg_dump \ test_rbtree \ test_rls_hooks \ - test_session_hooks \ test_shm_mq \ worker_spi diff --git a/src/test/modules/test_session_hooks/.gitignore b/src/test/modules/test_session_hooks/.gitignore deleted file mode 100644 index 5dcb3ff9723..00000000000 --- 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 index 636ae61c0e1..00000000000 --- a/src/test/modules/test_session_hooks/Makefile +++ /dev/null @@ -1,25 +0,0 @@ -# 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 - -# override installcheck - this module requires preloading the test module -installcheck: - @echo Cannot run $@ for test_session_hooks. Run "'make check'" instead. diff --git a/src/test/modules/test_session_hooks/README b/src/test/modules/test_session_hooks/README deleted file mode 100644 index 9cb42020c64..00000000000 --- a/src/test/modules/test_session_hooks/README +++ /dev/null @@ -1,2 +0,0 @@ -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 deleted file mode 100644 index be1b94953c5..00000000000 --- a/src/test/modules/test_session_hooks/expected/test_session_hooks.out +++ /dev/null @@ -1,31 +0,0 @@ -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 deleted file mode 100644 index fc62b4adef0..00000000000 --- 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 index 5e0864753d9..00000000000 --- a/src/test/modules/test_session_hooks/sql/test_session_hooks.sql +++ /dev/null @@ -1,12 +0,0 @@ -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--1.0.sql b/src/test/modules/test_session_hooks/test_session_hooks--1.0.sql deleted file mode 100644 index 16bcee9882e..00000000000 --- a/src/test/modules/test_session_hooks/test_session_hooks--1.0.sql +++ /dev/null @@ -1,4 +0,0 @@ -/* 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 deleted file mode 100644 index 4e2eef183e4..00000000000 --- a/src/test/modules/test_session_hooks/test_session_hooks.c +++ /dev/null @@ -1,134 +0,0 @@ -/* ------------------------------------------------------------------------- - * - * 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');", - dbname, username, 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() -{ - /* Hook just normal backends */ - if (MyBackendId != InvalidBackendId) - { - (void) register_session_hook("START"); - - if (prev_session_start_hook) - prev_session_start_hook(); - } -} - -/* sample session end hook function */ -static void -sample_session_end_hook() -{ - /* Hook just normal backends */ - if (MyBackendId != InvalidBackendId) - { - if (prev_session_end_hook) - prev_session_end_hook(); - - (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; -} diff --git a/src/test/modules/test_session_hooks/test_session_hooks.control b/src/test/modules/test_session_hooks/test_session_hooks.control deleted file mode 100644 index 7d7ef9f3f4a..00000000000 --- a/src/test/modules/test_session_hooks/test_session_hooks.control +++ /dev/null @@ -1,3 +0,0 @@ -comment = 'Test start/end hook session with an extension' -default_version = '1.0' -relocatable = true diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index 41f7832e5a6..719fe830476 100644 --- a/src/tools/msvc/vcregress.pl +++ b/src/tools/msvc/vcregress.pl @@ -383,8 +383,6 @@ sub modulescheck my $mstat = 0; foreach my $module (glob("*")) { - # test_session_hooks can't run installcheck, so skip it here - next if $module eq 'test_session_hooks'; subdircheck("$topdir/src/test/modules", $module); my $status = $? >> 8; $mstat ||= $status;