refactor CreateTrigger and CreateTriggerFiringOn

discussion: https://postgr.es/m/

Author: jianhe-fun

src/backend/catalog/index.c

@@ -2039,10 +2039,11 @@ index_constraint_create(Relation heapRelation,
 		trigger->deferrable = true;
 		trigger->initdeferred = initdeferred;
 		trigger->constrrel = NULL;
+		trigger->transformed = true;
 
 		(void) CreateTrigger(trigger, NULL, RelationGetRelid(heapRelation),
 							 InvalidOid, conOid, indexRelationId, InvalidOid,
-							 InvalidOid, NULL, true, false);
+							 InvalidOid, true, false);
 	}
 
 	/*

src/backend/commands/tablecmds.c

@@ -13826,10 +13826,11 @@ CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint,
 	fk_trigger->deferrable = fkconstraint->deferrable;
 	fk_trigger->initdeferred = fkconstraint->initdeferred;
 	fk_trigger->constrrel = NULL;
+	fk_trigger->transformed = true;
 
 	trigAddress = CreateTrigger(fk_trigger, NULL, myRelOid, refRelOid,
 								constraintOid, indexOid, InvalidOid,
-								parentTrigOid, NULL, true, false);
+								parentTrigOid, true, false);
 
 	/* Make changes-so-far visible */
 	CommandCounterIncrement();
@@ -13871,6 +13872,7 @@ createForeignKeyActionTriggers(Oid myRelOid, Oid refRelOid, Constraint *fkconstr
 	fk_trigger->whenClause = NULL;
 	fk_trigger->transitionRels = NIL;
 	fk_trigger->constrrel = NULL;
+	fk_trigger->transformed = true;
 
 	switch (fkconstraint->fk_del_action)
 	{
@@ -13907,7 +13909,7 @@ createForeignKeyActionTriggers(Oid myRelOid, Oid refRelOid, Constraint *fkconstr
 
 	trigAddress = CreateTrigger(fk_trigger, NULL, refRelOid, myRelOid,
 								constraintOid, indexOid, InvalidOid,
-								parentDelTrigger, NULL, true, false);
+								parentDelTrigger, true, false);
 	if (deleteTrigOid)
 		*deleteTrigOid = trigAddress.objectId;
 
@@ -13931,6 +13933,7 @@ createForeignKeyActionTriggers(Oid myRelOid, Oid refRelOid, Constraint *fkconstr
 	fk_trigger->whenClause = NULL;
 	fk_trigger->transitionRels = NIL;
 	fk_trigger->constrrel = NULL;
+	fk_trigger->transformed = true;
 
 	switch (fkconstraint->fk_upd_action)
 	{
@@ -13967,7 +13970,7 @@ createForeignKeyActionTriggers(Oid myRelOid, Oid refRelOid, Constraint *fkconstr
 
 	trigAddress = CreateTrigger(fk_trigger, NULL, refRelOid, myRelOid,
 								constraintOid, indexOid, InvalidOid,
-								parentUpdTrigger, NULL, true, false);
+								parentUpdTrigger, true, false);
 	if (updateTrigOid)
 		*updateTrigOid = trigAddress.objectId;
 }
@@ -20857,15 +20860,16 @@ CloneRowTriggersToPartition(Relation parent, Relation partition)
 		trigStmt->timing = trigForm->tgtype & TRIGGER_TYPE_TIMING_MASK;
 		trigStmt->events = trigForm->tgtype & TRIGGER_TYPE_EVENT_MASK;
 		trigStmt->columns = cols;
-		trigStmt->whenClause = NULL;	/* passed separately */
+		trigStmt->whenClause = qual;
 		trigStmt->transitionRels = NIL; /* not supported at present */
 		trigStmt->deferrable = trigForm->tgdeferrable;
 		trigStmt->initdeferred = trigForm->tginitdeferred;
 		trigStmt->constrrel = NULL; /* passed separately */
+		trigStmt->transformed = true; /* whenClause alerady transformed */
 
 		CreateTriggerFiringOn(trigStmt, NULL, RelationGetRelid(partition),
 							  trigForm->tgconstrrelid, InvalidOid, InvalidOid,
-							  trigForm->tgfoid, trigForm->oid, qual,
+							  trigForm->tgfoid, trigForm->oid,
 							  false, true, trigForm->tgenabled);
 
 		MemoryContextSwitchTo(oldcxt);

src/backend/commands/trigger.c

@@ -40,6 +40,7 @@
 #include "parser/parse_collate.h"
 #include "parser/parse_func.h"
 #include "parser/parse_relation.h"
+#include "parser/parse_utilcmd.h"
 #include "partitioning/partdesc.h"
 #include "pgstat.h"
 #include "rewrite/rewriteHandler.h"
@@ -139,9 +140,6 @@ static HeapTuple check_modified_virtual_generated(TupleDesc tupdesc, HeapTuple t
  * create the trigger on partitions, 2) when creating child foreign key
  * triggers; see CreateFKCheckTrigger() and createForeignKeyActionTriggers().
  *
- * If whenClause is passed, it is an already-transformed expression for
- * WHEN.  In this case, we ignore any that may come in stmt->whenClause.
- *
  * If isInternal is true then this is an internally-generated trigger.
  * This argument sets the tgisinternal field of the pg_trigger entry, and
  * if true causes us to modify the given trigger name to ensure uniqueness.
@@ -159,13 +157,13 @@ static HeapTuple check_modified_virtual_generated(TupleDesc tupdesc, HeapTuple t
 ObjectAddress
 CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
 			  Oid relOid, Oid refRelOid, Oid constraintOid, Oid indexOid,
-			  Oid funcoid, Oid parentTriggerOid, Node *whenClause,
+			  Oid funcoid, Oid parentTriggerOid,
 			  bool isInternal, bool in_partition)
 {
 	return
 		CreateTriggerFiringOn(stmt, queryString, relOid, refRelOid,
 							  constraintOid, indexOid, funcoid,
-							  parentTriggerOid, whenClause, isInternal,
+							  parentTriggerOid, isInternal,
 							  in_partition, TRIGGER_FIRES_ON_ORIGIN);
 }
 
@@ -177,15 +175,15 @@ ObjectAddress
 CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 					  Oid relOid, Oid refRelOid, Oid constraintOid,
 					  Oid indexOid, Oid funcoid, Oid parentTriggerOid,
-					  Node *whenClause, bool isInternal, bool in_partition,
+					  bool isInternal, bool in_partition,
 					  char trigger_fires_when)
 {
 	int16		tgtype;
 	int			ncolumns;
 	int16	   *columns;
 	int2vector *tgattr;
-	List	   *whenRtable;
-	char	   *qual;
+	List	   *whenRtable = NIL;
+	char	   *qual = NULL;
 	Datum		values[Natts_pg_trigger];
 	bool		nulls[Natts_pg_trigger];
 	Relation	rel;
@@ -207,6 +205,7 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 	Oid			existing_constraint_oid = InvalidOid;
 	bool		existing_isInternal = false;
 	bool		existing_isClone = false;
+	Node 		*whenClause = NULL;
 
 	if (OidIsValid(relOid))
 		rel = table_open(relOid, ShareRowExclusiveLock);
@@ -557,133 +556,21 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 	/*
 	 * Parse the WHEN clause, if any and we weren't passed an already
 	 * transformed one.
-	 *
-	 * Note that as a side effect, we fill whenRtable when parsing.  If we got
-	 * an already parsed clause, this does not occur, which is what we want --
-	 * no point in adding redundant dependencies below.
 	 */
-	if (!whenClause && stmt->whenClause)
+	if (stmt->whenClause)
 	{
-		ParseState *pstate;
-		ParseNamespaceItem *nsitem;
-		List	   *varList;
-		ListCell   *lc;
-
-		/* Set up a pstate to parse with */
-		pstate = make_parsestate(NULL);
-		pstate->p_sourcetext = queryString;
-
-		/*
-		 * Set up nsitems for OLD and NEW references.
-		 *
-		 * 'OLD' must always have varno equal to 1 and 'NEW' equal to 2.
-		 */
-		nsitem = addRangeTableEntryForRelation(pstate, rel,
-											   AccessShareLock,
-											   makeAlias("old", NIL),
-											   false, false);
-		addNSItemToQuery(pstate, nsitem, false, true, true);
-		nsitem = addRangeTableEntryForRelation(pstate, rel,
-											   AccessShareLock,
-											   makeAlias("new", NIL),
-											   false, false);
-		addNSItemToQuery(pstate, nsitem, false, true, true);
-
-		/* Transform expression.  Copy to be sure we don't modify original */
-		whenClause = transformWhereClause(pstate,
-										  copyObject(stmt->whenClause),
-										  EXPR_KIND_TRIGGER_WHEN,
-										  "WHEN");
-		/* we have to fix its collations too */
-		assign_expr_collations(pstate, whenClause);
-
-		/*
-		 * Check for disallowed references to OLD/NEW.
-		 *
-		 * NB: pull_var_clause is okay here only because we don't allow
-		 * subselects in WHEN clauses; it would fail to examine the contents
-		 * of subselects.
-		 */
-		varList = pull_var_clause(whenClause, 0);
-		foreach(lc, varList)
+		if (!stmt->transformed)
 		{
-			Var		   *var = (Var *) lfirst(lc);
+			stmt = transformCreateTriggerStmt(RelationGetRelid(rel), stmt,
+											  queryString);
 
-			switch (var->varno)
-			{
-				case PRS2_OLD_VARNO:
-					if (!TRIGGER_FOR_ROW(tgtype))
-						ereport(ERROR,
-								(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-								 errmsg("statement trigger's WHEN condition cannot reference column values"),
-								 parser_errposition(pstate, var->location)));
-					if (TRIGGER_FOR_INSERT(tgtype))
-						ereport(ERROR,
-								(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-								 errmsg("INSERT trigger's WHEN condition cannot reference OLD values"),
-								 parser_errposition(pstate, var->location)));
-					/* system columns are okay here */
-					break;
-				case PRS2_NEW_VARNO:
-					if (!TRIGGER_FOR_ROW(tgtype))
-						ereport(ERROR,
-								(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-								 errmsg("statement trigger's WHEN condition cannot reference column values"),
-								 parser_errposition(pstate, var->location)));
-					if (TRIGGER_FOR_DELETE(tgtype))
-						ereport(ERROR,
-								(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-								 errmsg("DELETE trigger's WHEN condition cannot reference NEW values"),
-								 parser_errposition(pstate, var->location)));
-					if (var->varattno < 0 && TRIGGER_FOR_BEFORE(tgtype))
-						ereport(ERROR,
-								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								 errmsg("BEFORE trigger's WHEN condition cannot reference NEW system columns"),
-								 parser_errposition(pstate, var->location)));
-					if (TRIGGER_FOR_BEFORE(tgtype) &&
-						var->varattno == 0 &&
-						RelationGetDescr(rel)->constr &&
-						(RelationGetDescr(rel)->constr->has_generated_stored ||
-						 RelationGetDescr(rel)->constr->has_generated_virtual))
-						ereport(ERROR,
-								(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-								 errmsg("BEFORE trigger's WHEN condition cannot reference NEW generated columns"),
-								 errdetail("A whole-row reference is used and the table contains generated columns."),
-								 parser_errposition(pstate, var->location)));
-					if (TRIGGER_FOR_BEFORE(tgtype) &&
-						var->varattno > 0 &&
-						TupleDescAttr(RelationGetDescr(rel), var->varattno - 1)->attgenerated)
-						ereport(ERROR,
-								(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-								 errmsg("BEFORE trigger's WHEN condition cannot reference NEW generated columns"),
-								 errdetail("Column \"%s\" is a generated column.",
-										   NameStr(TupleDescAttr(RelationGetDescr(rel), var->varattno - 1)->attname)),
-								 parser_errposition(pstate, var->location)));
-					break;
-				default:
-					/* can't happen without add_missing_from, so just elog */
-					elog(ERROR, "trigger WHEN condition cannot contain references to other relations");
-					break;
-			}
+			whenClause = stmt->whenClause;
+			Assert(whenClause != NULL);
 		}
-
-		/* we'll need the rtable for recordDependencyOnExpr */
-		whenRtable = pstate->p_rtable;
+		else
+			whenClause = stmt->whenClause;
 
 		qual = nodeToString(whenClause);
-
-		free_parsestate(pstate);
-	}
-	else if (!whenClause)
-	{
-		whenClause = NULL;
-		whenRtable = NIL;
-		qual = NULL;
-	}
-	else
-	{
-		qual = nodeToString(whenClause);
-		whenRtable = NIL;
 	}
 
 	/*
@@ -1129,10 +1016,40 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 	 * If it has a WHEN clause, add dependencies on objects mentioned in the
 	 * expression (eg, functions, as well as any columns used).
 	 */
-	if (whenRtable != NIL)
+	if (whenClause != NULL)
+	{
+		ParseState *pstate;
+		ParseNamespaceItem *nsitem;
+
+		/* Set up a pstate to parse with */
+		pstate = make_parsestate(NULL);
+		pstate->p_sourcetext = queryString;
+
+		/*
+		 * Set up nsitems for OLD and NEW references.
+		 *
+		 * 'OLD' must always have varno equal to 1 and 'NEW' equal to 2.
+		 */
+		nsitem = addRangeTableEntryForRelation(pstate, rel,
+											   AccessShareLock,
+											   makeAlias("old", NIL),
+											   false, false);
+		addNSItemToQuery(pstate, nsitem, false, true, true);
+
+		nsitem = addRangeTableEntryForRelation(pstate, rel,
+											   AccessShareLock,
+											   makeAlias("new", NIL),
+											   false, false);
+		addNSItemToQuery(pstate, nsitem, false, true, true);
+
+		/* we'll need the rtable for recordDependencyOnExpr */
+		whenRtable = pstate->p_rtable;
 		recordDependencyOnExpr(&myself, whenClause, whenRtable,
 							   DEPENDENCY_NORMAL);
 
+		free_parsestate(pstate);
+	}
+
 	/* Post creation hook for new trigger */
 	InvokeObjectPostCreateHookArg(TriggerRelationId, trigoid, 0,
 								  isInternal);
@@ -1175,7 +1092,6 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 			 */
 			childStmt = copyObject(stmt);
 			childStmt->funcname = NIL;
-			childStmt->whenClause = NULL;
 
 			/* If there is a WHEN clause, create a modified copy of it */
 			qual = copyObject(whenClause);
@@ -1185,11 +1101,13 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 			qual = (Node *)
 				map_partition_varattnos((List *) qual, PRS2_NEW_VARNO,
 										childTbl, rel);
+			childStmt->whenClause = qual;
+			childStmt->transformed = true;
 
 			CreateTriggerFiringOn(childStmt, queryString,
 								  partdesc->oids[i], refRelOid,
 								  InvalidOid, InvalidOid,
-								  funcoid, trigoid, qual,
+								  funcoid, trigoid,
 								  isInternal, true, trigger_fires_when);
 
 			table_close(childTbl, NoLock);

src/backend/parser/gram.y

@@ -6098,6 +6098,7 @@ CreateTrigStmt:
 					n->deferrable = false;
 					n->initdeferred = false;
 					n->constrrel = NULL;
+					n->transformed = false;
 					$$ = (Node *) n;
 				}
 		  | CREATE opt_or_replace CONSTRAINT TRIGGER name AFTER TriggerEvents ON
@@ -6148,6 +6149,7 @@ CreateTrigStmt:
 								   &n->deferrable, &n->initdeferred, &dummy,
 								   NULL, NULL, yyscanner);
 					n->constrrel = $10;
+					n->transformed = false;
 					$$ = (Node *) n;
 				}
 		;