From adc80ecc3f3be69126fc5079279138bb66fd6139 Mon Sep 17 00:00:00 2001 From: Deepakshi Mittal <78574784+deepakshi-mittal@users.noreply.github.com> Date: Wed, 3 Jan 2024 09:01:55 -0800 Subject: [PATCH] Support Instead of Trigger On Views (#272) Currently Postgres does not support Instead of Trigger On Views for DML queries. On running such a query on VIEW, query rewriting substitutes view with underlying base table. Added a hook for skipping the substitution for DML query on the VIEW. This change will add support for Instead of Triggers for DML queries when using Babelfish. Task: BABEL-2170/BABEL-4532/BABEL-4514 Signed-off-by: Deepakshi Mittal --- src/backend/commands/trigger.c | 5 ++++- src/backend/executor/execMain.c | 6 +++--- src/backend/executor/nodeModifyTable.c | 6 ++++-- src/backend/rewrite/rewriteHandler.c | 10 +++++++--- src/include/commands/trigger.h | 1 + src/include/executor/executor.h | 3 +++ 6 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 0feb9216520..040826651a5 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -72,6 +72,7 @@ int SessionReplicationRole = SESSION_REPLICATION_ROLE_ORIGIN; /* How many levels deep into trigger execution are we? */ static int MyTriggerDepth = 0; +List *triggerInvocationSequence = NIL; /* List to store all oids for triggers called*/ /* Local function prototypes */ static void renametrig_internal(Relation tgrel, Relation targetrel, @@ -484,7 +485,7 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString, RelationGetRelationName(rel)), errdetail("Triggers on foreign tables cannot have transition tables."))); - if (rel->rd_rel->relkind == RELKIND_VIEW) + if (rel->rd_rel->relkind == RELKIND_VIEW && sql_dialect != SQL_DIALECT_TSQL) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("\"%s\" is a view", @@ -2508,11 +2509,13 @@ ExecCallTriggerFunc(TriggerData *trigdata, MyTriggerDepth++; PG_TRY(); { + triggerInvocationSequence = lappend_oid(triggerInvocationSequence, trigdata->tg_trigger->tgoid); result = FunctionCallInvoke(fcinfo); } PG_FINALLY(); { MyTriggerDepth--; + triggerInvocationSequence = list_delete_last(triggerInvocationSequence); } PG_END_TRY(); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 6f1c738a0ef..af6ddefe325 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1061,7 +1061,7 @@ CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation) switch (operation) { case CMD_INSERT: - if (!trigDesc || (!trigDesc->trig_insert_instead_row)) + if (!trigDesc || (!trigDesc->trig_insert_instead_row && (sql_dialect == SQL_DIALECT_TSQL && !trigDesc->trig_insert_instead_statement))) ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("cannot insert into view \"%s\"", @@ -1069,7 +1069,7 @@ CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation) errhint("To enable inserting into the view, provide an INSTEAD OF INSERT trigger or an unconditional ON INSERT DO INSTEAD rule."))); break; case CMD_UPDATE: - if (!trigDesc || (!trigDesc->trig_update_instead_row)) + if (!trigDesc || (!trigDesc->trig_update_instead_row && (sql_dialect == SQL_DIALECT_TSQL && !trigDesc->trig_update_instead_statement))) ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("cannot update view \"%s\"", @@ -1077,7 +1077,7 @@ CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation) errhint("To enable updating the view, provide an INSTEAD OF UPDATE trigger or an unconditional ON UPDATE DO INSTEAD rule."))); break; case CMD_DELETE: - if (!trigDesc || (!trigDesc->trig_delete_instead_row)) + if (!trigDesc || (!trigDesc->trig_delete_instead_row && (sql_dialect == SQL_DIALECT_TSQL && !trigDesc->trig_delete_instead_statement))) ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("cannot delete from view \"%s\"", diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 92043dc54f0..cc896e67c3a 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -829,7 +829,8 @@ ExecInsert(ModifyTableContext *context, return NULL; /* "do nothing" */ } else if (sql_dialect == SQL_DIALECT_TSQL && resultRelInfo->ri_TrigDesc && - resultRelInfo->ri_TrigDesc->trig_insert_instead_statement && + resultRelInfo->ri_TrigDesc->trig_insert_instead_statement && + bbfViewHasInsteadofTrigger_hook && (bbfViewHasInsteadofTrigger_hook)(resultRelationDesc, CMD_INSERT) && isTsqlInsteadofTriggerExecution(estate, resultRelInfo, TRIGGER_EVENT_INSERT)) { ExecIRInsertTriggersTSQL(estate, resultRelInfo, slot, mtstate->mt_transition_capture); @@ -1502,6 +1503,7 @@ ExecDelete(ModifyTableContext *context, if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_delete_instead_statement && sql_dialect == SQL_DIALECT_TSQL && + bbfViewHasInsteadofTrigger_hook && (bbfViewHasInsteadofTrigger_hook)(resultRelationDesc, CMD_DELETE) && isTsqlInsteadofTriggerExecution(estate, resultRelInfo, TRIGGER_EVENT_DELETE)) { ExecIRDeleteTriggersTSQL(estate, resultRelInfo, tupleid, oldtuple, context->mtstate->mt_transition_capture); @@ -2354,7 +2356,7 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo, if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_update_instead_statement && - sql_dialect == SQL_DIALECT_TSQL && + sql_dialect == SQL_DIALECT_TSQL && bbfViewHasInsteadofTrigger_hook && (bbfViewHasInsteadofTrigger_hook)(resultRelationDesc, CMD_UPDATE) && isTsqlInsteadofTriggerExecution(estate, resultRelInfo, TRIGGER_EVENT_INSTEAD)) { ExecIRUpdateTriggersTSQL(estate, resultRelInfo, tupleid, oldtuple, slot, context->mtstate->mt_transition_capture); diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index dc211b1bb53..d6325f7e6f4 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -33,7 +33,7 @@ #include "nodes/nodeFuncs.h" #include "optimizer/optimizer.h" #include "parser/analyze.h" -#include "parser/parser.h" +#include "parser/parser.h" #include "parser/parse_coerce.h" #include "parser/parse_relation.h" #include "parser/parsetree.h" @@ -46,6 +46,8 @@ #include "utils/lsyscache.h" #include "utils/rel.h" +bbfViewHasInsteadofTrigger_hook_type bbfViewHasInsteadofTrigger_hook = NULL; /** BBF Hook to check Instead Of trigger on View */ + /* We use a list of these to detect recursion in RewriteQuery */ typedef struct rewrite_event { @@ -1479,7 +1481,8 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti, */ isAutoUpdatableView = false; if (target_relation->rd_rel->relkind == RELKIND_VIEW && - !view_has_instead_trigger(target_relation, CMD_INSERT)) + (!view_has_instead_trigger(target_relation, CMD_INSERT) && + !(sql_dialect == SQL_DIALECT_TSQL && bbfViewHasInsteadofTrigger_hook && (bbfViewHasInsteadofTrigger_hook)(target_relation, CMD_INSERT)))) { List *locks; bool hasUpdate; @@ -3945,7 +3948,8 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length) */ if (!instead && rt_entry_relation->rd_rel->relkind == RELKIND_VIEW && - !view_has_instead_trigger(rt_entry_relation, event)) + (!view_has_instead_trigger(rt_entry_relation, event) + && !(sql_dialect == SQL_DIALECT_TSQL && bbfViewHasInsteadofTrigger_hook && (bbfViewHasInsteadofTrigger_hook)(rt_entry_relation, event)))) { /* * If there were any qualified INSTEAD rules, don't allow the view diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 59ad3e437da..b0346bf9de6 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -291,6 +291,7 @@ extern void AfterTriggerEndSubXact(bool isCommit); extern void AfterTriggerSetState(ConstraintsSetStmt *stmt); extern bool AfterTriggerPendingOnRel(Oid relid); +extern List *triggerInvocationSequence; /* List to store all oids for triggers called */ /* * in utils/adt/ri_triggers.c diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 189ce1c947f..35ab4852cbd 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -99,6 +99,9 @@ extern PGDLLIMPORT ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook; typedef bool (*TriggerRecuresiveCheck_hook_type) (ResultRelInfo *resultRelInfo); extern PGDLLEXPORT TriggerRecuresiveCheck_hook_type TriggerRecuresiveCheck_hook; +typedef bool (*bbfViewHasInsteadofTrigger_hook_type) (Relation view, CmdType event); +extern PGDLLIMPORT bbfViewHasInsteadofTrigger_hook_type bbfViewHasInsteadofTrigger_hook; + typedef bool (*check_rowcount_hook_type) (int es_processed); extern PGDLLEXPORT check_rowcount_hook_type check_rowcount_hook;