BABEL: fix the bug instead of trigger behaves wrongly in recursively …

…call (yugabyte#31) recursively call the instead of trigger may lose the last DML to the triggered table in previous impl, recursively trigger a instead of trigger will not do any changes into the triggered table, as for the last triggered DML, it shouldn't do the instead of procedure, what it should do is to do DML in the table, since the last DML in a recursive trig chain should not trig it's trigger Task: BABEL-3115 & BABEL-3116 & BABEL-3118 Signed-off-by: Zhibai Song <[email protected]> Co-authored-by: Zhibai Song <[email protected]> (cherry picked from commit 0a65a6039e60fbe1017689f9b91ffdc28875719d)
abhinab-yb · Nov 14, 2024 · d034044 · d034044
1 parent be3a935
commit d034044
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 2 deletions.
diff --git a/src/postgres/src/backend/commands/trigger.c b/src/postgres/src/backend/commands/trigger.c
@@ -5384,7 +5384,7 @@ isTsqlInsteadofTriggerExecution(EState *estate, ResultRelInfo *relinfo, TriggerE
 	{
 		Trigger *trigger = &trigdesc->triggers[i];
 		if (TriggerEnabled(estate, relinfo, trigger, event, NULL, NULL, NULL)){
-			return true;
+			return !TsqlRecuresiveCheck(relinfo);
 		}
 	}
 	return false;
@@ -7394,6 +7394,12 @@ void BeginCompositeTriggers(MemoryContext curCxt)
 	compositeTriggers.curCxt = curCxt;
 }
 
+bool TsqlRecuresiveCheck(ResultRelInfo *resultRelInfo){
+	if (TriggerRecuresiveCheck_hook)
+		return TriggerRecuresiveCheck_hook(resultRelInfo);
+	else return false;
+}
+
 /*
  * End current nesting level for trigger from TSQL executor
  * If there is trigger data at current level, fire all trigger

diff --git a/src/postgres/src/backend/executor/execMain.c b/src/postgres/src/backend/executor/execMain.c
@@ -77,6 +77,7 @@ ExecutorRun_hook_type ExecutorRun_hook = NULL;
 ExecutorFinish_hook_type ExecutorFinish_hook = NULL;
 ExecutorEnd_hook_type ExecutorEnd_hook = NULL;
 
+TriggerRecuresiveCheck_hook_type TriggerRecuresiveCheck_hook = NULL;
 /* Hook for plugin to get control in ExecCheckRTPerms() */
 ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook = NULL;
 

diff --git a/src/postgres/src/backend/executor/nodeModifyTable.c b/src/postgres/src/backend/executor/nodeModifyTable.c
@@ -927,9 +927,11 @@ 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 && !TsqlRecuresiveCheck(resultRelInfo)){
 		ExecIRInsertTriggersTSQL(estate, resultRelInfo, slot, mtstate->mt_transition_capture);
 		// if it's a statement level IOT trigger, only get the transition table
+		if (canSetTag)
+			(estate->es_processed)++;
 		return NULL;
 	}
 	else if (resultRelInfo->ri_FdwRoutine)
@@ -1585,6 +1587,8 @@ ExecDelete(ModifyTableContext *context,
 		isTsqlInsteadofTriggerExecution(estate, resultRelInfo, TRIGGER_EVENT_DELETE))
 	{
 		ExecIRDeleteTriggersTSQL(estate, resultRelInfo, tupleid, oldtuple, context->mtstate->mt_transition_capture);
+		if (canSetTag)
+			(estate->es_processed)++;
 		return NULL;
 	}
 
@@ -2722,6 +2726,8 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
 		isTsqlInsteadofTriggerExecution(estate, resultRelInfo, TRIGGER_EVENT_INSTEAD))
 	{
 		ExecIRUpdateTriggersTSQL(estate, resultRelInfo, tupleid, oldtuple, slot, recheckIndexes, context->mtstate->mt_transition_capture);
+		if (canSetTag)
+			(estate->es_processed)++;
 		return NULL;
 	}
 

diff --git a/src/postgres/src/include/commands/trigger.h b/src/postgres/src/include/commands/trigger.h
@@ -320,4 +320,6 @@ extern bool HasNonRITrigger(const TriggerDesc* trigDesc);
 extern void BeginCompositeTriggers(MemoryContext curCxt);
 extern void EndCompositeTriggers(bool error);
 
+extern bool TsqlRecuresiveCheck(ResultRelInfo *resultRelInfo);
+
 #endif							/* TRIGGER_H */
diff --git a/src/postgres/src/include/executor/executor.h b/src/postgres/src/include/executor/executor.h
@@ -93,6 +93,8 @@ extern PGDLLIMPORT ExecutorEnd_hook_type ExecutorEnd_hook;
 typedef bool (*ExecutorCheckPerms_hook_type) (List *, bool);
 extern PGDLLIMPORT ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook;
 
+typedef bool (*TriggerRecuresiveCheck_hook_type) (ResultRelInfo *resultRelInfo);
+extern PGDLLIMPORT TriggerRecuresiveCheck_hook_type TriggerRecuresiveCheck_hook;
 
 /*
  * prototypes from functions in execAmi.c

diff --git a/src/postgres/src/include/utils/guc.h b/src/postgres/src/include/utils/guc.h
@@ -279,6 +279,8 @@ extern PGDLLIMPORT double log_xact_sample_rate;
 extern PGDLLIMPORT char *backtrace_functions;
 extern PGDLLIMPORT char *backtrace_symbol_list;
 
+extern bool pltsql_recursive_triggers;
+
 extern PGDLLIMPORT int temp_file_limit;
 
 extern PGDLLIMPORT int num_temp_buffers;

diff --git a/src/postgres/src/tools/pgindent/typedefs.list b/src/postgres/src/tools/pgindent/typedefs.list
@@ -675,6 +675,7 @@ ExecStatus
 ExecStatusType
 ExecuteStmt
 ExecutorCheckPerms_hook_type
+TriggerRecuresiveCheck_hook_type
 ExecutorEnd_hook_type
 ExecutorFinish_hook_type
 ExecutorRun_hook_type