[CF 5782] v1 - foreign key on virtual generated column

This branch was automatically generated by a robot using patches from an
email thread registered at:

https://commitfest.postgresql.org/patch/5782

The branch will be overwritten each time a new patch version is posted to
the thread, and also periodically to check for bitrot caused by changes
on the master branch.

Patch(es): https://www.postgresql.org/message-id/CACJufxEfzfERVwoUaFODz9AOabDOLkNkfTbajB82dSJVuTQQOw@mail.gmail.com
Author(s): Jian He

Author: Commitfest Bot

src/backend/commands/copyfrom.c

@@ -1346,8 +1346,9 @@ CopyFrom(CopyFromState cstate)
 				/* Compute stored generated columns */
 				if (resultRelInfo->ri_RelationDesc->rd_att->constr &&
 					resultRelInfo->ri_RelationDesc->rd_att->constr->has_generated_stored)
-					ExecComputeStoredGenerated(resultRelInfo, estate, myslot,
-											   CMD_INSERT);
+					ExecComputeGenerated(resultRelInfo, estate, myslot,
+										 CMD_INSERT,
+										 false);
 
 				/*
 				 * If the target is a plain table, check the constraints of

src/backend/commands/tablecmds.c

@@ -659,7 +659,7 @@ static void RebuildConstraintComment(AlteredTableInfo *tab, AlterTablePass pass,
 									 Oid objid, Relation rel, List *domname,
 									 const char *conname);
 static void TryReuseIndex(Oid oldId, IndexStmt *stmt);
-static void TryReuseForeignKey(Oid oldId, Constraint *con);
+static void TryReuseForeignKey(Oid oldId, Constraint *con, Oid oldRelId);
 static ObjectAddress ATExecAlterColumnGenericOptions(Relation rel, const char *colName,
 													 List *options, LOCKMODE lockmode);
 static void change_owner_fix_column_acls(Oid relationOid,
@@ -8642,17 +8642,17 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName,
 		 * this renders them pointless.
 		 */
 		RelationClearMissing(rel);
+	}
 
-		/* make sure we don't conflict with later attribute modifications */
-		CommandCounterIncrement();
+	/* make sure we don't conflict with later attribute modifications */
+	CommandCounterIncrement();
 
-		/*
-		 * Find everything that depends on the column (constraints, indexes,
-		 * etc), and record enough information to let us recreate the objects
-		 * after rewrite.
-		 */
-		RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName);
-	}
+	/*
+	 * Find everything that depends on the column (constraints, indexes,
+	 * etc), and record enough information to let us recreate the objects
+	 * after rewrite.
+	*/
+	RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName);
 
 	/*
 	 * Drop the dependency records of the GENERATED expression, in particular
@@ -10222,19 +10222,6 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 						 errmsg("invalid %s action for foreign key constraint containing generated column",
 								"ON DELETE")));
 		}
-
-		/*
-		 * FKs on virtual columns are not supported.  This would require
-		 * various additional support in ri_triggers.c, including special
-		 * handling in ri_NullCheck(), ri_KeysEqual(),
-		 * RI_FKey_fk_upd_check_required() (since all virtual columns appear
-		 * as NULL there).  Also not really practical as long as you can't
-		 * index virtual columns.
-		 */
-		if (attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("foreign key constraints on virtual generated columns are not supported")));
 	}
 
 	/*
@@ -15665,7 +15652,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
 					/* rewriting neither side of a FK */
 					if (con->contype == CONSTR_FOREIGN &&
 						!rewrite && tab->rewrite == 0)
-						TryReuseForeignKey(oldId, con);
+						TryReuseForeignKey(oldId, con, oldRelId);
 					con->reset_default_tblspc = true;
 					cmd->subtype = AT_ReAddConstraint;
 					tab->subcmds[AT_PASS_OLD_CONSTR] =
@@ -15822,16 +15809,23 @@ TryReuseIndex(Oid oldId, IndexStmt *stmt)
  * Stash the old P-F equality operator into the Constraint node, for possible
  * use by ATAddForeignKeyConstraint() in determining whether revalidation of
  * this constraint can be skipped.
+ *
+ * oldId is the old (previous) foreign key pg_constraint oid.
+ * oldRelId: the relation where this foreign key constraint is on.
+ * revalidation can not be skipped if any foreign key attribute is virtual
+ * generated column.
  */
 static void
-TryReuseForeignKey(Oid oldId, Constraint *con)
+TryReuseForeignKey(Oid oldId, Constraint *con, Oid oldRelId)
 {
 	HeapTuple	tup;
-	Datum		adatum;
-	ArrayType  *arr;
-	Oid		   *rawarr;
 	int			numkeys;
 	int			i;
+	Relation	rel;
+	Oid			conpfeqop[INDEX_MAX_KEYS];
+	AttrNumber	conkey[INDEX_MAX_KEYS];
+	AttrNumber	confkey[INDEX_MAX_KEYS];
+	bool		fkey_on_virtual_generated = false;
 
 	Assert(con->contype == CONSTR_FOREIGN);
 	Assert(con->old_conpfeqop == NIL);	/* already prepared this node */
@@ -15840,20 +15834,41 @@ TryReuseForeignKey(Oid oldId, Constraint *con)
 	if (!HeapTupleIsValid(tup)) /* should not happen */
 		elog(ERROR, "cache lookup failed for constraint %u", oldId);
 
-	adatum = SysCacheGetAttrNotNull(CONSTROID, tup,
-									Anum_pg_constraint_conpfeqop);
-	arr = DatumGetArrayTypeP(adatum);	/* ensure not toasted */
-	numkeys = ARR_DIMS(arr)[0];
-	/* test follows the one in ri_FetchConstraintInfo() */
-	if (ARR_NDIM(arr) != 1 ||
-		ARR_HASNULL(arr) ||
-		ARR_ELEMTYPE(arr) != OIDOID)
-		elog(ERROR, "conpfeqop is not a 1-D Oid array");
-	rawarr = (Oid *) ARR_DATA_PTR(arr);
+	DeconstructFkConstraintRow(tup,
+							   &numkeys,
+							   conkey,
+							   confkey,
+							   conpfeqop,
+							   NULL, 	/* pp_eq_oprs */
+							   NULL, 	/* ff_eq_oprs */
+							   NULL, 	/* num_fk_del_set_cols */
+							   NULL); 	/* fk_del_set_cols */
+
+	rel = table_open(oldRelId, NoLock);
+
+	if (rel->rd_att->constr &&
+		rel->rd_att->constr->has_generated_virtual)
+	{
+		TupleDesc	tupleDesc = RelationGetDescr(rel);
+
+		for (i = 0; i < numkeys; i++)
+		{
+			Form_pg_attribute attr = TupleDescAttr(tupleDesc, conkey[i] - 1);
+			if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+			{
+				fkey_on_virtual_generated = true;
+				break;
+			}
+		}
+	}
+	table_close(rel, NoLock);
 
 	/* stash a List of the operator Oids in our Constraint node */
-	for (i = 0; i < numkeys; i++)
-		con->old_conpfeqop = lappend_oid(con->old_conpfeqop, rawarr[i]);
+	if (!fkey_on_virtual_generated)
+	{
+		for (i = 0; i < numkeys; i++)
+			con->old_conpfeqop = lappend_oid(con->old_conpfeqop, conpfeqop[i]);
+	}
 
 	ReleaseSysCache(tup);
 }

src/backend/executor/execReplication.c

@@ -587,8 +587,9 @@ ExecSimpleRelationInsert(ResultRelInfo *resultRelInfo,
 		/* Compute stored generated columns */
 		if (rel->rd_att->constr &&
 			rel->rd_att->constr->has_generated_stored)
-			ExecComputeStoredGenerated(resultRelInfo, estate, slot,
-									   CMD_INSERT);
+			ExecComputeGenerated(resultRelInfo, estate, slot,
+								 CMD_INSERT,
+								 false);
 
 		/* Check the constraints of the tuple */
 		if (rel->rd_att->constr)
@@ -684,8 +685,9 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo,
 		/* Compute stored generated columns */
 		if (rel->rd_att->constr &&
 			rel->rd_att->constr->has_generated_stored)
-			ExecComputeStoredGenerated(resultRelInfo, estate, slot,
-									   CMD_UPDATE);
+			ExecComputeGenerated(resultRelInfo, estate, slot,
+								 CMD_UPDATE,
+								 false);
 
 		/* Check the constraints of the tuple */
 		if (rel->rd_att->constr)

src/backend/executor/execUtils.c

@@ -1404,7 +1404,7 @@ ExecGetExtraUpdatedCols(ResultRelInfo *relinfo, EState *estate)
 {
 	/* Compute the info if we didn't already */
 	if (!relinfo->ri_extraUpdatedCols_valid)
-		ExecInitGenerated(relinfo, estate, CMD_UPDATE);
+		ExecInitGenerated(relinfo, estate, CMD_UPDATE, false);
 	return relinfo->ri_extraUpdatedCols;
 }
 

src/backend/executor/nodeModifyTable.c

@@ -415,7 +415,8 @@ ExecCheckTIDVisible(EState *estate,
  *
  * This fills the resultRelInfo's ri_GeneratedExprsI/ri_NumGeneratedNeededI or
  * ri_GeneratedExprsU/ri_NumGeneratedNeededU fields, depending on cmdtype.
- * This is used only for stored generated columns.
+ * This is mainly used only for stored generated columns. However if
+ * compute_virtual is true, we do the same for virtual generated column.
  *
  * If cmdType == CMD_UPDATE, the ri_extraUpdatedCols field is filled too.
  * This is used by both stored and virtual generated columns.
@@ -428,7 +429,8 @@ ExecCheckTIDVisible(EState *estate,
 void
 ExecInitGenerated(ResultRelInfo *resultRelInfo,
 				  EState *estate,
-				  CmdType cmdtype)
+				  CmdType cmdtype,
+				  bool compute_virtual)
 {
 	Relation	rel = resultRelInfo->ri_RelationDesc;
 	TupleDesc	tupdesc = RelationGetDescr(rel);
@@ -472,10 +474,15 @@ ExecInitGenerated(ResultRelInfo *resultRelInfo,
 			Expr	   *expr;
 
 			/* Fetch the GENERATED AS expression tree */
-			expr = (Expr *) build_column_default(rel, i + 1);
-			if (expr == NULL)
-				elog(ERROR, "no generation expression found for column number %d of table \"%s\"",
-					 i + 1, RelationGetRelationName(rel));
+			if (attgenerated == ATTRIBUTE_GENERATED_STORED)
+			{
+				expr = (Expr *) build_column_default(rel, i + 1);
+				if (expr == NULL)
+					elog(ERROR, "no generation expression found for column number %d of table \"%s\"",
+						i + 1, RelationGetRelationName(rel));
+			}
+			else
+				expr = (Expr *) build_generation_expression(rel, i+1);
 
 			/*
 			 * If it's an update with a known set of update target columns,
@@ -497,6 +504,11 @@ ExecInitGenerated(ResultRelInfo *resultRelInfo,
 				ri_GeneratedExprs[i] = ExecPrepareExpr(expr, estate);
 				ri_NumGeneratedNeeded++;
 			}
+			else if (compute_virtual)
+			{
+				ri_GeneratedExprs[i] = ExecPrepareExpr(expr, estate);
+				ri_NumGeneratedNeeded++;
+			}
 
 			/* If UPDATE, mark column in resultRelInfo->ri_extraUpdatedCols */
 			if (cmdtype == CMD_UPDATE)
@@ -537,12 +549,14 @@ ExecInitGenerated(ResultRelInfo *resultRelInfo,
 }
 
 /*
- * Compute stored generated columns for a tuple
+ * Compute generated columns for a tuple.
+ * If compute_virtual is true, we exclusively compute virtual generated columns.
+ * We do not compute stored and virtual generated columns simultaneously.
  */
 void
-ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
-						   EState *estate, TupleTableSlot *slot,
-						   CmdType cmdtype)
+ExecComputeGenerated(ResultRelInfo *resultRelInfo, EState *estate,
+					 TupleTableSlot *slot, CmdType cmdtype,
+					 bool compute_virtual)
 {
 	Relation	rel = resultRelInfo->ri_RelationDesc;
 	TupleDesc	tupdesc = RelationGetDescr(rel);
@@ -554,7 +568,9 @@ ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
 	bool	   *nulls;
 
 	/* We should not be called unless this is true */
-	Assert(tupdesc->constr && tupdesc->constr->has_generated_stored);
+	Assert(tupdesc->constr);
+	Assert(tupdesc->constr->has_generated_stored ||
+		   tupdesc->constr->has_generated_virtual);
 
 	/*
 	 * Initialize the expressions if we didn't already, and check whether we
@@ -563,15 +579,15 @@ ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
 	if (cmdtype == CMD_UPDATE)
 	{
 		if (resultRelInfo->ri_GeneratedExprsU == NULL)
-			ExecInitGenerated(resultRelInfo, estate, cmdtype);
+			ExecInitGenerated(resultRelInfo, estate, cmdtype, compute_virtual);
 		if (resultRelInfo->ri_NumGeneratedNeededU == 0)
 			return;
 		ri_GeneratedExprs = resultRelInfo->ri_GeneratedExprsU;
 	}
 	else
 	{
 		if (resultRelInfo->ri_GeneratedExprsI == NULL)
-			ExecInitGenerated(resultRelInfo, estate, cmdtype);
+			ExecInitGenerated(resultRelInfo, estate, cmdtype, compute_virtual);
 		/* Early exit is impossible given the prior Assert */
 		Assert(resultRelInfo->ri_NumGeneratedNeededI > 0);
 		ri_GeneratedExprs = resultRelInfo->ri_GeneratedExprsI;
@@ -594,7 +610,7 @@ ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
 			Datum		val;
 			bool		isnull;
 
-			Assert(TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_STORED);
+			Assert(TupleDescAttr(tupdesc, i)->attgenerated != '\0');
 
 			econtext->ecxt_scantuple = slot;
 
@@ -931,8 +947,9 @@ ExecInsert(ModifyTableContext *context,
 		 */
 		if (resultRelationDesc->rd_att->constr &&
 			resultRelationDesc->rd_att->constr->has_generated_stored)
-			ExecComputeStoredGenerated(resultRelInfo, estate, slot,
-									   CMD_INSERT);
+			ExecComputeGenerated(resultRelInfo, estate, slot,
+								 CMD_INSERT,
+								 false);
 
 		/*
 		 * If the FDW supports batching, and batching is requested, accumulate
@@ -1058,8 +1075,9 @@ ExecInsert(ModifyTableContext *context,
 		 */
 		if (resultRelationDesc->rd_att->constr &&
 			resultRelationDesc->rd_att->constr->has_generated_stored)
-			ExecComputeStoredGenerated(resultRelInfo, estate, slot,
-									   CMD_INSERT);
+			ExecComputeGenerated(resultRelInfo, estate, slot,
+								 CMD_INSERT,
+								 false);
 
 		/*
 		 * Check any RLS WITH CHECK policies.
@@ -2146,8 +2164,9 @@ ExecUpdatePrepareSlot(ResultRelInfo *resultRelInfo,
 	 */
 	if (resultRelationDesc->rd_att->constr &&
 		resultRelationDesc->rd_att->constr->has_generated_stored)
-		ExecComputeStoredGenerated(resultRelInfo, estate, slot,
-								   CMD_UPDATE);
+		ExecComputeGenerated(resultRelInfo, estate, slot,
+							 CMD_UPDATE,
+							 false);
 }
 
 /*