From 4355ca494541903fe8b1f2abaed29c4f8357f959 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Wed, 14 Oct 2020 13:56:58 -0700 Subject: [PATCH] trigger fix and tests --- Makefile | 2 +- cstore_customscan.c | 2 +- cstore_tableam.c | 59 +++++++++++++++++++++++++++++++++++++ expected/am_trigger.out | 65 +++++++++++++++++++++++++++++++++++++++++ sql/am_trigger.sql | 61 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 187 insertions(+), 2 deletions(-) create mode 100644 expected/am_trigger.out create mode 100644 sql/am_trigger.sql diff --git a/Makefile b/Makefile index 7e8bee13a..0d581f145 100644 --- a/Makefile +++ b/Makefile @@ -54,7 +54,7 @@ ifeq ($(USE_TABLEAM),yes) OBJS += cstore_tableam.o cstore_customscan.o REGRESS += am_create am_load am_query am_analyze am_data_types am_functions \ am_drop am_insert am_copyto am_alter am_rollback am_truncate am_vacuum am_clean \ - am_block_filtering am_join + am_block_filtering am_join am_trigger ISOLATION += am_vacuum_vs_insert endif diff --git a/cstore_customscan.c b/cstore_customscan.c index 0dcdff111..d7e6eb667 100644 --- a/cstore_customscan.c +++ b/cstore_customscan.c @@ -145,7 +145,7 @@ CStoreSetRelPathlistHook(PlannerInfo *root, RelOptInfo *rel, Index rti, return; } - if (!OidIsValid(rte->relid)) + if (!OidIsValid(rte->relid) || rte->rtekind != RTE_RELATION) { /* some calls to the pathlist hook don't have a valid relation set. Do nothing */ return; diff --git a/cstore_tableam.c b/cstore_tableam.c index eae806e59..09a65d75b 100644 --- a/cstore_tableam.c +++ b/cstore_tableam.c @@ -16,6 +16,7 @@ #include "catalog/index.h" #include "catalog/objectaccess.h" #include "catalog/pg_am.h" +#include "catalog/pg_trigger.h" #include "catalog/storage.h" #include "catalog/storage_xlog.h" #include "commands/progress.h" @@ -31,6 +32,7 @@ #include "storage/predicate.h" #include "storage/procarray.h" #include "storage/smgr.h" +#include "tcop/utility.h" #include "utils/builtins.h" #include "utils/pg_rusage.h" #include "utils/rel.h" @@ -62,11 +64,19 @@ static TableWriteState *CStoreWriteState = NULL; static ExecutorEnd_hook_type PreviousExecutorEndHook = NULL; static MemoryContext CStoreContext = NULL; static object_access_hook_type prevObjectAccessHook = NULL; +static ProcessUtility_hook_type PreviousProcessUtilityHook = NULL; /* forward declaration for static functions */ static void CStoreTableAMObjectAccessHook(ObjectAccessType access, Oid classId, Oid objectId, int subId, void *arg); +static void CStoreTableAMProcessUtility(PlannedStmt *plannedStatement, + const char *queryString, + ProcessUtilityContext context, + ParamListInfo paramListInfo, + QueryEnvironment *queryEnvironment, + DestReceiver *destReceiver, + char *completionTag); static bool IsCStoreTableAmTable(Oid relationId); static bool ConditionalLockRelationWithTimeout(Relation rel, LOCKMODE lockMode, int timeout, int retryInterval); @@ -1027,11 +1037,60 @@ CStoreExecutorEnd(QueryDesc *queryDesc) } +static void +CStoreTableAMProcessUtility(PlannedStmt *plannedStatement, + const char *queryString, + ProcessUtilityContext context, + ParamListInfo paramListInfo, + QueryEnvironment *queryEnvironment, + DestReceiver *destReceiver, + char *completionTag) +{ + Node *parseTree = plannedStatement->utilityStmt; + + if (nodeTag(parseTree) == T_CreateTrigStmt) + { + CreateTrigStmt *createTrigStmt = (CreateTrigStmt *) parseTree; + Relation rel; + bool isCStore; + + rel = relation_openrv(createTrigStmt->relation, AccessShareLock); + isCStore = rel->rd_tableam == GetCstoreTableAmRoutine(); + relation_close(rel, AccessShareLock); + + if (isCStore && + createTrigStmt->row && + createTrigStmt->timing == TRIGGER_TYPE_AFTER) + { + ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg( + "AFTER ROW triggers are not supported for columnstore access method"), + errhint("Consider an AFTER STATEMENT trigger instead."))); + } + } + + if (PreviousProcessUtilityHook != NULL) + { + PreviousProcessUtilityHook(plannedStatement, queryString, context, + paramListInfo, queryEnvironment, + destReceiver, completionTag); + } + else + { + standard_ProcessUtility(plannedStatement, queryString, context, + paramListInfo, queryEnvironment, + destReceiver, completionTag); + } +} + + void cstore_tableam_init() { PreviousExecutorEndHook = ExecutorEnd_hook; ExecutorEnd_hook = CStoreExecutorEnd; + PreviousProcessUtilityHook = ProcessUtility_hook; + ProcessUtility_hook = CStoreTableAMProcessUtility; prevObjectAccessHook = object_access_hook; object_access_hook = CStoreTableAMObjectAccessHook; diff --git a/expected/am_trigger.out b/expected/am_trigger.out new file mode 100644 index 000000000..53b2c9d9e --- /dev/null +++ b/expected/am_trigger.out @@ -0,0 +1,65 @@ +create or replace function trs_before() returns trigger language plpgsql as $$ +BEGIN + RAISE NOTICE 'BEFORE STATEMENT %', TG_OP; + RETURN NULL; +END; +$$; +create or replace function trs_after() returns trigger language plpgsql as $$ +DECLARE + r RECORD; +BEGIN + RAISE NOTICE 'AFTER STATEMENT %', TG_OP; + IF (TG_OP = 'DELETE') THEN + FOR R IN select * from old_table + LOOP + RAISE NOTICE ' (%)', r.i; + END LOOP; + ELSE + FOR R IN select * from new_table + LOOP + RAISE NOTICE ' (%)', r.i; + END LOOP; + END IF; + RETURN NULL; +END; +$$; +create or replace function trr_before() returns trigger language plpgsql as $$ +BEGIN + RAISE NOTICE 'BEFORE ROW %: (%)', TG_OP, NEW.i; + RETURN NEW; +END; +$$; +create or replace function trr_after() returns trigger language plpgsql as $$ +BEGIN + RAISE NOTICE 'AFTER ROW %: (%)', TG_OP, NEW.i; + RETURN NEW; +END; +$$; +create table test_tr(i int) using cstore_tableam; +create trigger tr_before_stmt before insert on test_tr + for each statement execute procedure trs_before(); +create trigger tr_after_stmt after insert on test_tr + referencing new table as new_table + for each statement execute procedure trs_after(); +create trigger tr_before_row before insert on test_tr + for each row execute procedure trr_before(); +-- after triggers require TIDs, which are not supported yet +create trigger tr_after_row after insert on test_tr + for each row execute procedure trr_after(); +ERROR: AFTER ROW triggers are not supported for columnstore access method +HINT: Consider an AFTER STATEMENT trigger instead. +insert into test_tr values(1); +NOTICE: BEFORE STATEMENT INSERT +NOTICE: BEFORE ROW INSERT: (1) +NOTICE: AFTER STATEMENT INSERT +NOTICE: (1) +insert into test_tr values(2),(3),(4); +NOTICE: BEFORE STATEMENT INSERT +NOTICE: BEFORE ROW INSERT: (2) +NOTICE: BEFORE ROW INSERT: (3) +NOTICE: BEFORE ROW INSERT: (4) +NOTICE: AFTER STATEMENT INSERT +NOTICE: (2) +NOTICE: (3) +NOTICE: (4) +drop table test_tr; diff --git a/sql/am_trigger.sql b/sql/am_trigger.sql new file mode 100644 index 000000000..b8a918cf4 --- /dev/null +++ b/sql/am_trigger.sql @@ -0,0 +1,61 @@ + +create or replace function trs_before() returns trigger language plpgsql as $$ +BEGIN + RAISE NOTICE 'BEFORE STATEMENT %', TG_OP; + RETURN NULL; +END; +$$; + +create or replace function trs_after() returns trigger language plpgsql as $$ +DECLARE + r RECORD; +BEGIN + RAISE NOTICE 'AFTER STATEMENT %', TG_OP; + IF (TG_OP = 'DELETE') THEN + FOR R IN select * from old_table + LOOP + RAISE NOTICE ' (%)', r.i; + END LOOP; + ELSE + FOR R IN select * from new_table + LOOP + RAISE NOTICE ' (%)', r.i; + END LOOP; + END IF; + RETURN NULL; +END; +$$; + +create or replace function trr_before() returns trigger language plpgsql as $$ +BEGIN + RAISE NOTICE 'BEFORE ROW %: (%)', TG_OP, NEW.i; + RETURN NEW; +END; +$$; + +create or replace function trr_after() returns trigger language plpgsql as $$ +BEGIN + RAISE NOTICE 'AFTER ROW %: (%)', TG_OP, NEW.i; + RETURN NEW; +END; +$$; + +create table test_tr(i int) using cstore_tableam; + +create trigger tr_before_stmt before insert on test_tr + for each statement execute procedure trs_before(); +create trigger tr_after_stmt after insert on test_tr + referencing new table as new_table + for each statement execute procedure trs_after(); + +create trigger tr_before_row before insert on test_tr + for each row execute procedure trr_before(); + +-- after triggers require TIDs, which are not supported yet +create trigger tr_after_row after insert on test_tr + for each row execute procedure trr_after(); + +insert into test_tr values(1); +insert into test_tr values(2),(3),(4); + +drop table test_tr;