On Thu, Mar 20, 2025 at 12:19 AM Álvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
>
> Other comments:
>
> * The block in ATRewriteTable that creates a resultRelInfo for
>   ExecRelCheckGenVirtualNotNull needs an explanation.
>
I tried my best.
here are the comments above the line ``if (notnull_virtual_attrs != NIL)``

        /*
         * Whether change an existing generation expression or adding a new
         * not-null virtual generated column, we need to evaluate whether the
         * generation expression is null.
         * In ExecConstraints, we use ExecRelCheckGenVirtualNotNull do the job.
         * Here, we can also utilize ExecRelCheckGenVirtualNotNull.
         * To achieve this, we need create a dummy ResultRelInfo.  Ensure that
         * the resultRelationIndex of the dummy ResultRelInfo is set to 0.
        */


+ /*
+ * XXX this deserves an explanation. Also, is rInfo a good variable
+ * name?
+ */
there are other places using this variable name: "rInfo", so i use
these names....


ATRewriteTable:
        for (i = 0; i < newTupDesc->natts; i++)
        {
            Form_pg_attribute attr = TupleDescAttr(newTupDesc, i);
            if (attr->attnotnull && !attr->attisdropped)
            {
                if (attr->attgenerated != ATTRIBUTE_GENERATED_VIRTUAL)
                    notnull_attrs = lappend_int(notnull_attrs, i);
                else
                    notnull_virtual_attrs = lappend_int(notnull_virtual_attrs,
                                                        attr->attnum);
            }
        }
this is kind of ugly? notnull_virtual_attrs is 1 based, notnull_attrs
is 0 based.
I want to change it all to 1 based. see v5-0002


> * I suspect the locations for the new functions were selected with
>   the help of a dartboard.  ExecRelCheckGenVirtualNotNull() in
>   particular looks like it could use a better location.  Maybe it's
>   better right after ExecConstraints, and ReportNotNullViolationError
>   (or whatever we name it) can go after it.

I thought ExecRelCheckGenVirtualNotNull is very similar to ExecRelCheck,
that's why I put it close to ExecRelCheck.
putting it right after ExecConstraints is ok for me.


> * I don't find the name all_virtual_nns particularly appropriate.  Maybe
>   virtual_notnull_attrs?
>
in ATRewriteTable we already have "notnull_virtual_attrs"
we can rename it to notnull_virtual_attrs


> * There are some funny rules around nulls on rowtypes.  I think allowing
>   this tuple is wrong (and I left an XXX comment near where the 'argisrow'
>   flag is set):
>
> create type twoints as (a int, b int);
> create table foo (a int, b int, c twoints generated always as 
> (row(a,b)::twoints) not null);
> insert into foo values (null, null);
>
> I don't remember exactly what the rules are though so I may be wrong.
>
argisrow should set to false.
i think you mean the special value ``'(,)'``

create table t(a twoints not null);
insert into t select '(,)' returning a is null; --return true;
create table t1(a twoints not null generated always as ('(,)'::twoints));
insert into t1 default values returning a is null; --should return true.

i think you mean this thread:
https://postgr.es/m/173591158454.714.7664064332419606...@wrigleys.postgresql.org
should i put a test into generated_virtual.sql?


+ * We implement this by consing up a NullTest node for each virtual
trivial question.
 I googled, and still found any explanation of the word "consing up".
From e7086ce90253907cc186e78c913670876af0bff3 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Thu, 20 Mar 2025 11:59:55 +0800
Subject: [PATCH v5 1/2] not null for virtual generated column
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

now we can add not null constraint on virtual generated column.
not null constraint on virtual generated column make sure evaulation of the
expanded generated expression does not yield null.
the virtual generated columns does not store  value, the storage is null.

we do support ALTER COLUMN SET EXPRESSION over not null virtual generated column.

reviewed by: Xuneng Zhou <xunengzhou@gmail.com>,
reviewed by: Navneet Kumar <thanit3111@gmail.com>,
reviewed by: Álvaro Herrera <alvherre@alvh.no-ip.org>
discussion: https://postgr.es/m/CACJufxHArQysbDkWFmvK+D1TPHQWWTxWN15cMuUaTYX3xhQXgg@mail.gmail.com
---
 src/backend/catalog/heap.c                    |  10 -
 src/backend/commands/indexcmds.c              |  10 +-
 src/backend/commands/tablecmds.c              |  72 +++++-
 src/backend/executor/execMain.c               | 244 ++++++++++++++----
 src/backend/parser/parse_utilcmd.c            |  14 -
 src/include/executor/executor.h               |   4 +
 src/include/nodes/execnodes.h                 |   3 +
 .../regress/expected/generated_virtual.out    |  89 +++++--
 src/test/regress/sql/generated_virtual.sql    |  50 +++-
 9 files changed, 373 insertions(+), 123 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index bd3554c0bfd..b807ab66668 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2615,11 +2615,6 @@ AddRelationNewConstraints(Relation rel,
 						errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 						errmsg("cannot add not-null constraint on system column \"%s\"",
 							   strVal(linitial(cdef->keys))));
-			/* TODO: see transformColumnDefinition() */
-			if (get_attgenerated(RelationGetRelid(rel), colnum) == ATTRIBUTE_GENERATED_VIRTUAL)
-				ereport(ERROR,
-						errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						errmsg("not-null constraints are not supported on virtual generated columns"));
 
 			/*
 			 * If the column already has a not-null constraint, we don't want
@@ -2935,11 +2930,6 @@ AddRelationNotNullConstraints(Relation rel, List *constraints,
 					errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					errmsg("cannot add not-null constraint on system column \"%s\"",
 						   strVal(linitial(constr->keys))));
-		/* TODO: see transformColumnDefinition() */
-		if (get_attgenerated(RelationGetRelid(rel), attnum) == ATTRIBUTE_GENERATED_VIRTUAL)
-			ereport(ERROR,
-					errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					errmsg("not-null constraints are not supported on virtual generated columns"));
 
 		/*
 		 * A column can only have one not-null constraint, so discard any
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 89cc83e8843..33c2106c17c 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1118,10 +1118,12 @@ DefineIndex(Oid tableId,
 
 		if (TupleDescAttr(RelationGetDescr(rel), attno - 1)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
 			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 stmt->isconstraint ?
-					 errmsg("unique constraints on virtual generated columns are not supported") :
-					 errmsg("indexes on virtual generated columns are not supported")));
+					errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					stmt->primary ?
+					errmsg("primary keys on virtual generated columns are not supported") :
+					stmt->isconstraint ?
+					errmsg("unique constraints on virtual generated columns are not supported") :
+					errmsg("indexes on virtual generated columns are not supported"));
 	}
 
 	/*
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 129c97fdf28..0751c0d627c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6092,6 +6092,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
 	TupleDesc	newTupDesc;
 	bool		needscan = false;
 	List	   *notnull_attrs;
+	List	   *notnull_virtual_attrs;
 	int			i;
 	ListCell   *l;
 	EState	   *estate;
@@ -6176,22 +6177,30 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
 		ex->exprstate = ExecInitExpr((Expr *) ex->expr, NULL);
 	}
 
-	notnull_attrs = NIL;
+	notnull_attrs = notnull_virtual_attrs = NIL;
 	if (newrel || tab->verify_new_notnull)
 	{
 		/*
 		 * If we are rebuilding the tuples OR if we added any new but not
 		 * verified not-null constraints, check all not-null constraints. This
 		 * is a bit of overkill but it minimizes risk of bugs.
+		 *
+		 * Collect attribute numbers on virtual generated columns separately!
 		 */
 		for (i = 0; i < newTupDesc->natts; i++)
 		{
 			Form_pg_attribute attr = TupleDescAttr(newTupDesc, i);
 
 			if (attr->attnotnull && !attr->attisdropped)
-				notnull_attrs = lappend_int(notnull_attrs, i);
+			{
+				if (attr->attgenerated != ATTRIBUTE_GENERATED_VIRTUAL)
+					notnull_attrs = lappend_int(notnull_attrs, i);
+				else
+					notnull_virtual_attrs = lappend_int(notnull_virtual_attrs,
+														attr->attnum);
+			}
 		}
-		if (notnull_attrs)
+		if (notnull_attrs || notnull_virtual_attrs)
 			needscan = true;
 	}
 
@@ -6205,6 +6214,32 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
 		List	   *dropped_attrs = NIL;
 		ListCell   *lc;
 		Snapshot	snapshot;
+		ResultRelInfo *rInfo = NULL;
+
+		/*
+		 * Whether change an existing generation expression or adding a new
+		 * not-null virtual generated column, we need to evaluate whether the
+		 * generation expression is null.
+		 * In ExecConstraints, we use ExecRelCheckGenVirtualNotNull do the job.
+		 * Here, we can also utilize ExecRelCheckGenVirtualNotNull.
+		 * To achieve this, we need create a dummy ResultRelInfo.  Ensure that
+		 * the resultRelationIndex of the dummy ResultRelInfo is set to 0.
+		*/
+		if (notnull_virtual_attrs != NIL)
+		{
+			MemoryContext oldcontext;
+
+			Assert(newTupDesc->constr->has_generated_virtual);
+			Assert(newTupDesc->constr->has_not_null);
+			oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
+			rInfo = makeNode(ResultRelInfo);
+			InitResultRelInfo(rInfo,
+							  oldrel,
+							  0,	/* dummy rangetable index */
+							  NULL,
+							  estate->es_instrument);
+			MemoryContextSwitchTo(oldcontext);
+		}
 
 		if (newrel)
 			ereport(DEBUG1,
@@ -6387,6 +6422,26 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
 				}
 			}
 
+			if (notnull_virtual_attrs != NIL)
+			{
+				AttrNumber	attnum;
+
+				attnum = ExecRelCheckGenVirtualNotNull(rInfo, insertslot,
+													   estate,
+													   notnull_virtual_attrs);
+				if (attnum != InvalidAttrNumber)
+				{
+					Form_pg_attribute attr = TupleDescAttr(newTupDesc, attnum - 1);
+
+					ereport(ERROR,
+							errcode(ERRCODE_NOT_NULL_VIOLATION),
+							errmsg("column \"%s\" of relation \"%s\" contains null values",
+								   NameStr(attr->attname),
+								   RelationGetRelationName(oldrel)),
+							errtablecol(oldrel, attnum));
+				}
+			}
+
 			foreach(l, tab->constraints)
 			{
 				NewConstraint *con = lfirst(l);
@@ -7836,14 +7891,6 @@ ATExecSetNotNull(List **wqueue, Relation rel, char *conName, char *colName,
 				 errmsg("cannot alter system column \"%s\"",
 						colName)));
 
-	/* TODO: see transformColumnDefinition() */
-	if (TupleDescAttr(RelationGetDescr(rel), attnum - 1)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("not-null constraints are not supported on virtual generated columns"),
-				 errdetail("Column \"%s\" of relation \"%s\" is a virtual generated column.",
-						   colName, RelationGetRelationName(rel))));
-
 	/* See if there's already a constraint */
 	tuple = findNotNullConstraintAttnum(RelationGetRelid(rel), attnum);
 	if (HeapTupleIsValid(tuple))
@@ -8512,6 +8559,9 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName,
 				 errdetail("Column \"%s\" of relation \"%s\" is a virtual generated column.",
 						   colName, RelationGetRelationName(rel))));
 
+	if (attgenerated == ATTRIBUTE_GENERATED_VIRTUAL && attTup->attnotnull)
+		tab->verify_new_notnull = true;
+
 	/*
 	 * We need to prevent this because a change of expression could affect a
 	 * row filter and inject expressions that are not permitted in a row
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index e9bd98c7738..3ad2d35db4e 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -51,6 +51,7 @@
 #include "foreign/fdwapi.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
+#include "nodes/makefuncs.h"
 #include "nodes/queryjumble.h"
 #include "parser/parse_relation.h"
 #include "pgstat.h"
@@ -92,7 +93,9 @@ static bool ExecCheckPermissionsModified(Oid relOid, Oid userid,
 										 AclMode requiredPerms);
 static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt);
 static void EvalPlanQualStart(EPQState *epqstate, Plan *planTree);
-
+static void ReportNotNullViolationError(ResultRelInfo *resultRelInfo,
+										TupleTableSlot *slot,
+										EState *estate, int attrChk);
 /* end of local decls */
 
 
@@ -1372,6 +1375,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
 	resultRelInfo->ri_FdwState = NULL;
 	resultRelInfo->ri_usesFdwDirectModify = false;
 	resultRelInfo->ri_ConstraintExprs = NULL;
+	resultRelInfo->ri_GeneratedNotNullExprs = NULL;
 	resultRelInfo->ri_GeneratedExprsI = NULL;
 	resultRelInfo->ri_GeneratedExprsU = NULL;
 	resultRelInfo->ri_projectReturning = NULL;
@@ -2058,73 +2062,61 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
 	TupleDesc	tupdesc = RelationGetDescr(rel);
 	TupleConstr *constr = tupdesc->constr;
 	Bitmapset  *modifiedCols;
+	List	   *notnull_virtual_attrs = NIL;
+	bool		virtual_notnull_check = false;
 
 	Assert(constr);				/* we should not be called otherwise */
 
+	/*
+	 * First, go over all the not-null constraints and verify and possibly
+	 * throw errors for all of those that aren't on virtual generated columns.
+	 * (The latter need executor support and a precise list of columns to
+	 * handle, so we accumulate that here and initialize separately.)
+	 */
 	if (constr->has_not_null)
 	{
-		int			natts = tupdesc->natts;
-		int			attrChk;
-
-		for (attrChk = 1; attrChk <= natts; attrChk++)
+		for (AttrNumber attnum = 1; attnum <= tupdesc->natts; attnum++)
 		{
-			Form_pg_attribute att = TupleDescAttr(tupdesc, attrChk - 1);
+			Form_pg_attribute att = TupleDescAttr(tupdesc, attnum - 1);
 
-			if (att->attnotnull && slot_attisnull(slot, attrChk))
+			if (att->attnotnull && att->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+				notnull_virtual_attrs = lappend_int(notnull_virtual_attrs, att->attnum);
+
+			if (att->attnotnull && slot_attisnull(slot, attnum))
 			{
-				char	   *val_desc;
-				Relation	orig_rel = rel;
-				TupleDesc	orig_tupdesc = RelationGetDescr(rel);
-
-				/*
-				 * If the tuple has been routed, it's been converted to the
-				 * partition's rowtype, which might differ from the root
-				 * table's.  We must convert it back to the root table's
-				 * rowtype so that val_desc shown error message matches the
-				 * input tuple.
-				 */
-				if (resultRelInfo->ri_RootResultRelInfo)
+				if (att->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
 				{
-					ResultRelInfo *rootrel = resultRelInfo->ri_RootResultRelInfo;
-					AttrMap    *map;
-
-					tupdesc = RelationGetDescr(rootrel->ri_RelationDesc);
-					/* a reverse map */
-					map = build_attrmap_by_name_if_req(orig_tupdesc,
-													   tupdesc,
-													   false);
-
-					/*
-					 * Partition-specific slot's tupdesc can't be changed, so
-					 * allocate a new one.
-					 */
-					if (map != NULL)
-						slot = execute_attr_map_slot(map, slot,
-													 MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
-					modifiedCols = bms_union(ExecGetInsertedCols(rootrel, estate),
-											 ExecGetUpdatedCols(rootrel, estate));
-					rel = rootrel->ri_RelationDesc;
+					if (!virtual_notnull_check)
+						virtual_notnull_check = true;
+					continue;
 				}
-				else
-					modifiedCols = bms_union(ExecGetInsertedCols(resultRelInfo, estate),
-											 ExecGetUpdatedCols(resultRelInfo, estate));
-				val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
-														 slot,
-														 tupdesc,
-														 modifiedCols,
-														 64);
 
-				ereport(ERROR,
-						(errcode(ERRCODE_NOT_NULL_VIOLATION),
-						 errmsg("null value in column \"%s\" of relation \"%s\" violates not-null constraint",
-								NameStr(att->attname),
-								RelationGetRelationName(orig_rel)),
-						 val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
-						 errtablecol(orig_rel, attrChk)));
+				ReportNotNullViolationError(resultRelInfo, slot, estate, attnum);
 			}
 		}
 	}
 
+	/* Check not-null constraints on virtual generated column, if any */
+	if (virtual_notnull_check)
+	{
+		AttrNumber	attnum;
+
+		Assert(constr->has_not_null);
+		Assert(constr->has_generated_virtual);
+		Assert(notnull_virtual_attrs != NIL);
+
+		attnum = ExecRelCheckGenVirtualNotNull(resultRelInfo, slot, estate,
+											   notnull_virtual_attrs);
+		if (attnum != InvalidAttrNumber)
+		{
+			Form_pg_attribute att = TupleDescAttr(tupdesc, attnum - 1);
+
+			Assert(att->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL);
+			ReportNotNullViolationError(resultRelInfo, slot, estate, att->attnum);
+		}
+	}
+
+	/* Last, verify any CHECK constraints */
 	if (rel->rd_rel->relchecks > 0)
 	{
 		const char *failed;
@@ -2134,7 +2126,12 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
 			char	   *val_desc;
 			Relation	orig_rel = rel;
 
-			/* See the comment above. */
+			/*
+			 * If the tuple has been routed, it's been converted to the
+			 * partition's rowtype, which might differ from the root table's.
+			 * We must convert it back to the root table's rowtype so that
+			 * val_desc shown error message matches the input tuple.
+			 */
 			if (resultRelInfo->ri_RootResultRelInfo)
 			{
 				ResultRelInfo *rootrel = resultRelInfo->ri_RootResultRelInfo;
@@ -2176,6 +2173,145 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
 	}
 }
 
+/*
+ * Verify not-null constraints on virtual generated columns of the given
+ * tuple slot.
+ *
+ * Return value of InvalidAttrNumber means all not-null constraints on virtual
+ * generated columns are satisfied.  A return value > 0 means a not-null
+ * violation happened for that attribute.
+ *
+ * notnull_virtual_attrs is the list of the attnums of virtual generated column with
+ * not-null constraints.
+ */
+AttrNumber
+ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
+							  EState *estate, List *notnull_virtual_attrs)
+{
+	Relation	rel = resultRelInfo->ri_RelationDesc;
+	ExprContext *econtext;
+	MemoryContext oldContext;
+
+	/*
+	 * We implement this by consing up a NullTest node for each virtual
+	 * generated column, which we cache in resultRelInfo, and running those
+	 * through ExecCheck.
+	 */
+	if (resultRelInfo->ri_GeneratedNotNullExprs == NULL)
+	{
+		int			cnt = list_length(notnull_virtual_attrs);
+
+		oldContext = MemoryContextSwitchTo(estate->es_query_cxt);
+		resultRelInfo->ri_GeneratedNotNullExprs =
+			(ExprState **) palloc0(cnt * sizeof(ExprState *));
+
+		foreach_int(attnum, notnull_virtual_attrs)
+		{
+			int			i = foreach_current_index(attnum);
+			NullTest   *nnulltest;
+
+			nnulltest = makeNode(NullTest);
+			nnulltest->arg = (Expr *) build_generation_expression(rel, attnum);
+			nnulltest->nulltesttype = IS_NOT_NULL;
+			nnulltest->argisrow = false;
+			nnulltest->location = -1;
+
+			resultRelInfo->ri_GeneratedNotNullExprs[i] =
+				ExecPrepareExpr((Expr *) nnulltest, estate);
+		}
+		MemoryContextSwitchTo(oldContext);
+	}
+
+	/*
+	 * We will use the EState's per-tuple context for evaluating virtual
+	 * generated column not null constraint expressions (creating it if it's
+	 * not already there).
+	 */
+	econtext = GetPerTupleExprContext(estate);
+
+	/* Arrange for econtext's scan tuple to be the tuple under test */
+	econtext->ecxt_scantuple = slot;
+
+	/* And evaluate the check constraints for virtual generated column */
+	foreach_int(attnum, notnull_virtual_attrs)
+	{
+		int			i = foreach_current_index(attnum);
+		ExprState  *exprstate = resultRelInfo->ri_GeneratedNotNullExprs[i];
+
+		Assert(exprstate != NULL);
+		if (!ExecCheck(exprstate, econtext))
+			return attnum;
+	}
+
+	/* InvalidAttrNumber result means no error */
+	return InvalidAttrNumber;
+}
+
+/*
+ * Report a violation of a not-null constraint that was already detected.
+ */
+static void
+ReportNotNullViolationError(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
+							EState *estate, int attrChk)
+{
+	Bitmapset  *modifiedCols;
+	char	   *val_desc;
+
+	Relation	rel = resultRelInfo->ri_RelationDesc;
+	Relation	orig_rel = rel;
+
+	TupleDesc	tupdesc = RelationGetDescr(rel);
+	TupleDesc	orig_tupdesc = RelationGetDescr(rel);
+	Form_pg_attribute att = TupleDescAttr(tupdesc, attrChk - 1);
+
+	Assert(attrChk > 0);
+
+	/*
+	 * If the tuple has been routed, it's been converted to the partition's
+	 * rowtype, which might differ from the root table's.  We must convert it
+	 * back to the root table's rowtype so that val_desc shown error message
+	 * matches the input tuple.
+	 */
+	if (resultRelInfo->ri_RootResultRelInfo)
+	{
+		ResultRelInfo *rootrel = resultRelInfo->ri_RootResultRelInfo;
+		AttrMap    *map;
+
+		tupdesc = RelationGetDescr(rootrel->ri_RelationDesc);
+		/* a reverse map */
+		map = build_attrmap_by_name_if_req(orig_tupdesc,
+										   tupdesc,
+										   false);
+
+		/*
+		 * Partition-specific slot's tupdesc can't be changed, so allocate a
+		 * new one.
+		 */
+		if (map != NULL)
+			slot = execute_attr_map_slot(map, slot,
+										 MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
+		modifiedCols = bms_union(ExecGetInsertedCols(rootrel, estate),
+								 ExecGetUpdatedCols(rootrel, estate));
+		rel = rootrel->ri_RelationDesc;
+	}
+	else
+		modifiedCols = bms_union(ExecGetInsertedCols(resultRelInfo, estate),
+								 ExecGetUpdatedCols(resultRelInfo, estate));
+
+	val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+											 slot,
+											 tupdesc,
+											 modifiedCols,
+											 64);
+	ereport(ERROR,
+			errcode(ERRCODE_NOT_NULL_VIOLATION),
+			errmsg("null value in column \"%s\" of relation \"%s\" violates not-null constraint",
+				   NameStr(att->attname),
+				   RelationGetRelationName(orig_rel)),
+			val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
+			errtablecol(orig_rel, attrChk));
+}
+
 /*
  * ExecWithCheckOptions -- check that tuple satisfies any WITH CHECK OPTIONs
  * of the specified kind.
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 896a7f2c59b..9c1541e1fea 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -988,20 +988,6 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
 							column->colname, cxt->relation->relname),
 					 parser_errposition(cxt->pstate,
 										constraint->location)));
-
-		/*
-		 * TODO: Straightforward not-null constraints won't work on virtual
-		 * generated columns, because there is no support for expanding the
-		 * column when the constraint is checked.  Maybe we could convert the
-		 * not-null constraint into a full check constraint, so that the
-		 * generation expression can be expanded at check time.
-		 */
-		if (column->is_not_null && column->generated == ATTRIBUTE_GENERATED_VIRTUAL)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("not-null constraints are not supported on virtual generated columns"),
-					 parser_errposition(cxt->pstate,
-										constraint->location)));
 	}
 
 	/*
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 0db5d18ba22..7a2525e8ce1 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -220,6 +220,10 @@ extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid,
 extern List *ExecGetAncestorResultRels(EState *estate, ResultRelInfo *resultRelInfo);
 extern void ExecConstraints(ResultRelInfo *resultRelInfo,
 							TupleTableSlot *slot, EState *estate);
+extern AttrNumber ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo,
+												TupleTableSlot *slot,
+												EState *estate,
+												List *notnull_virtual_attrs);
 extern bool ExecPartitionCheck(ResultRelInfo *resultRelInfo,
 							   TupleTableSlot *slot, EState *estate, bool emitError);
 extern void ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index d4d4e655180..6159b8a3c07 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -549,6 +549,9 @@ typedef struct ResultRelInfo
 	/* array of constraint-checking expr states */
 	ExprState **ri_ConstraintExprs;
 
+	/* array of virtual generated not null constraint-checking expr states */
+	ExprState **ri_GeneratedNotNullExprs;
+
 	/*
 	 * Arrays of stored generated columns ExprStates for INSERT/UPDATE/MERGE.
 	 */
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index dc09c85938e..98f2e486f88 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -664,28 +664,68 @@ INSERT INTO gtest20c VALUES (1);  -- ok
 INSERT INTO gtest20c VALUES (NULL);  -- fails
 ERROR:  new row for relation "gtest20c" violates check constraint "whole_row_check"
 DETAIL:  Failing row contains (null, virtual).
--- not-null constraints (currently not supported)
 CREATE TABLE gtest21a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL NOT NULL);
-ERROR:  not-null constraints are not supported on virtual generated columns
-LINE 1: ... b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL NOT NULL);
-                                                             ^
---INSERT INTO gtest21a (a) VALUES (1);  -- ok
---INSERT INTO gtest21a (a) VALUES (0);  -- violates constraint
+INSERT INTO gtest21a (a) VALUES (1);  -- ok
+INSERT INTO gtest21a (a) VALUES (0);  -- violates constraint
+ERROR:  null value in column "b" of relation "gtest21a" violates not-null constraint
+DETAIL:  Failing row contains (0, virtual).
 -- also check with table constraint syntax
-CREATE TABLE gtest21ax (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL, CONSTRAINT cc NOT NULL b);  -- error
-ERROR:  not-null constraints are not supported on virtual generated columns
+CREATE TABLE gtest21ax (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL, CONSTRAINT cc NOT NULL b);
+INSERT INTO gtest21ax (a) VALUES (0);  -- violates constraint
+ERROR:  null value in column "b" of relation "gtest21ax" violates not-null constraint
+DETAIL:  Failing row contains (0, virtual).
+INSERT INTO gtest21ax (a) VALUES (1);  --ok
+-- SET EXPRESSION support not null constraint
+ALTER TABLE gtest21ax ALTER COLUMN b SET EXPRESSION AS (nullif(a, 1)); --error
+ERROR:  column "b" of relation "gtest21ax" contains null values
+DROP TABLE gtest21ax;
 CREATE TABLE gtest21ax (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL);
-ALTER TABLE gtest21ax ADD CONSTRAINT cc NOT NULL b;  -- error
-ERROR:  not-null constraints are not supported on virtual generated columns
+ALTER TABLE gtest21ax ADD CONSTRAINT cc NOT NULL b;
+INSERT INTO gtest21ax (a) VALUES (0);  -- violates constraint
+ERROR:  null value in column "b" of relation "gtest21ax" violates not-null constraint
+DETAIL:  Failing row contains (0, virtual).
 DROP TABLE gtest21ax;
-CREATE TABLE gtest21b (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL);
+CREATE TABLE gtest21b (a int, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL);
 ALTER TABLE gtest21b ALTER COLUMN b SET NOT NULL;
-ERROR:  not-null constraints are not supported on virtual generated columns
-DETAIL:  Column "b" of relation "gtest21b" is a virtual generated column.
---INSERT INTO gtest21b (a) VALUES (1);  -- ok
---INSERT INTO gtest21b (a) VALUES (0);  -- violates constraint
+INSERT INTO gtest21b (a) VALUES (1);  -- ok
+INSERT INTO gtest21b (a) VALUES (2), (0);  -- violates constraint
+ERROR:  null value in column "b" of relation "gtest21b" violates not-null constraint
+DETAIL:  Failing row contains (0, virtual).
+INSERT INTO gtest21b (a) VALUES (NULL);  -- error
+ERROR:  null value in column "b" of relation "gtest21b" violates not-null constraint
+DETAIL:  Failing row contains (null, virtual).
 ALTER TABLE gtest21b ALTER COLUMN b DROP NOT NULL;
---INSERT INTO gtest21b (a) VALUES (0);  -- ok now
+INSERT INTO gtest21b (a) VALUES (0);  -- ok now
+--not null constraint with partitioned table case.
+CREATE TABLE gtestnn_parent(f1 int, f2 bigint, f3 bigint GENERATED ALWAYS AS (nullif(f1, 1) + nullif(f2, 10)) VIRTUAL not null) PARTITION BY RANGE (f1);
+CREATE TABLE gtestnn_child PARTITION OF gtestnn_parent FOR VALUES FROM (1) TO (5);
+CREATE TABLE gtestnn_childdef PARTITION OF gtestnn_parent default;
+--check the error message.
+INSERT INTO gtestnn_parent VALUES (2, 2, default), (3, 5, default), (14, 12, default);
+INSERT INTO gtestnn_parent VALUES (1, 2, default); --error
+ERROR:  null value in column "f3" of relation "gtestnn_child" violates not-null constraint
+DETAIL:  Failing row contains (1, 2, virtual).
+INSERT INTO gtestnn_parent VALUES (2, 10, default); --error
+ERROR:  null value in column "f3" of relation "gtestnn_child" violates not-null constraint
+DETAIL:  Failing row contains (2, 10, virtual).
+ALTER TABLE gtestnn_parent ALTER COLUMN f3 SET EXPRESSION AS (nullif(f1, 2) + nullif(f2, 11)); --error
+ERROR:  column "f3" of relation "gtestnn_child" contains null values
+INSERT INTO gtestnn_parent VALUES (10, 11, default); --error
+select * from gtestnn_parent;
+ f1 | f2 | f3 
+----+----+----
+  2 |  2 |  4
+  3 |  5 |  8
+ 14 | 12 | 26
+ 10 | 11 | 21
+(4 rows)
+
+--test ALTER TABLE ADD COLUMN.
+ALTER TABLE gtestnn_parent ADD COLUMN c int NOT NULL GENERATED ALWAYS AS (nullif(f1, 14) + nullif(f2, 10)) VIRTUAL; --error
+ERROR:  column "c" of relation "gtestnn_childdef" contains null values
+ALTER TABLE gtestnn_parent ADD COLUMN c int NOT NULL GENERATED ALWAYS AS (nullif(f1, 13) + nullif(f2, 5)) VIRTUAL; --error
+ERROR:  column "c" of relation "gtestnn_child" contains null values
+ALTER TABLE gtestnn_parent ADD COLUMN c int NOT NULL GENERATED ALWAYS AS (nullif(f1, 4) + nullif(f2, 6)) VIRTUAL; --ok
 -- index constraints
 CREATE TABLE gtest22a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a / 2) VIRTUAL UNIQUE);
 ERROR:  unique constraints on virtual generated columns are not supported
@@ -693,7 +733,7 @@ ERROR:  unique constraints on virtual generated columns are not supported
 --INSERT INTO gtest22a VALUES (3);
 --INSERT INTO gtest22a VALUES (4);
 CREATE TABLE gtest22b (a int, b int GENERATED ALWAYS AS (a / 2) VIRTUAL, PRIMARY KEY (a, b));
-ERROR:  not-null constraints are not supported on virtual generated columns
+ERROR:  primary keys on virtual generated columns are not supported
 --INSERT INTO gtest22b VALUES (2);
 --INSERT INTO gtest22b VALUES (2);
 -- indexes
@@ -738,7 +778,7 @@ ERROR:  foreign key constraints on virtual generated columns are not supported
 --DROP TABLE gtest23b;
 --DROP TABLE gtest23a;
 CREATE TABLE gtest23p (x int, y int GENERATED ALWAYS AS (x * 2) VIRTUAL, PRIMARY KEY (y));
-ERROR:  not-null constraints are not supported on virtual generated columns
+ERROR:  primary keys on virtual generated columns are not supported
 --INSERT INTO gtest23p VALUES (1), (2), (3);
 CREATE TABLE gtest23q (a int PRIMARY KEY, b int REFERENCES gtest23p (y));
 ERROR:  relation "gtest23p" does not exist
@@ -1029,7 +1069,7 @@ CREATE TABLE gtest27 (
     b int,
     x int GENERATED ALWAYS AS ((a + b) * 2) VIRTUAL
 );
-INSERT INTO gtest27 (a, b) VALUES (3, 7), (4, 11);
+INSERT INTO gtest27 (a, b) VALUES (3, 7), (4, 11), (NULL, NULL);
 ALTER TABLE gtest27 ALTER COLUMN a TYPE text;  -- error
 ERROR:  cannot alter type of a column used by a generated column
 DETAIL:  Column "a" is used by generated column "x".
@@ -1047,7 +1087,8 @@ SELECT * FROM gtest27;
 ---+----+----
  3 |  7 | 20
  4 | 11 | 30
-(2 rows)
+   |    |   
+(3 rows)
 
 ALTER TABLE gtest27 ALTER COLUMN x TYPE boolean USING x <> 0;  -- error
 ERROR:  cannot specify USING when altering type of generated column
@@ -1056,6 +1097,14 @@ LINE 1: ALTER TABLE gtest27 ALTER COLUMN x TYPE boolean USING x <> 0...
 DETAIL:  Column "x" is a generated column.
 ALTER TABLE gtest27 ALTER COLUMN x DROP DEFAULT;  -- error
 ERROR:  column "x" of relation "gtest27" is a generated column
+--not null error violation
+ALTER TABLE gtest27
+  DROP COLUMN x,
+  ALTER COLUMN a TYPE bigint,
+  ALTER COLUMN b TYPE bigint,
+  ADD COLUMN x bigint GENERATED ALWAYS AS ((a + b) * 2) VIRTUAL NOT NULL;
+ERROR:  column "x" of relation "gtest27" contains null values
+DELETE FROM gtest27 WHERE a IS NULL AND b IS NULL;
 -- It's possible to alter the column types this way:
 ALTER TABLE gtest27
   DROP COLUMN x,
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index dab8c92ef99..485e2a317fe 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -335,23 +335,46 @@ ALTER TABLE gtest20c ADD CONSTRAINT whole_row_check CHECK (gtest20c IS NOT NULL)
 INSERT INTO gtest20c VALUES (1);  -- ok
 INSERT INTO gtest20c VALUES (NULL);  -- fails
 
--- not-null constraints (currently not supported)
 CREATE TABLE gtest21a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL NOT NULL);
---INSERT INTO gtest21a (a) VALUES (1);  -- ok
---INSERT INTO gtest21a (a) VALUES (0);  -- violates constraint
+INSERT INTO gtest21a (a) VALUES (1);  -- ok
+INSERT INTO gtest21a (a) VALUES (0);  -- violates constraint
 
 -- also check with table constraint syntax
-CREATE TABLE gtest21ax (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL, CONSTRAINT cc NOT NULL b);  -- error
+CREATE TABLE gtest21ax (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL, CONSTRAINT cc NOT NULL b);
+INSERT INTO gtest21ax (a) VALUES (0);  -- violates constraint
+INSERT INTO gtest21ax (a) VALUES (1);  --ok
+-- SET EXPRESSION support not null constraint
+ALTER TABLE gtest21ax ALTER COLUMN b SET EXPRESSION AS (nullif(a, 1)); --error
+DROP TABLE gtest21ax;
+
 CREATE TABLE gtest21ax (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL);
-ALTER TABLE gtest21ax ADD CONSTRAINT cc NOT NULL b;  -- error
+ALTER TABLE gtest21ax ADD CONSTRAINT cc NOT NULL b;
+INSERT INTO gtest21ax (a) VALUES (0);  -- violates constraint
 DROP TABLE gtest21ax;
 
-CREATE TABLE gtest21b (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL);
+CREATE TABLE gtest21b (a int, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL);
 ALTER TABLE gtest21b ALTER COLUMN b SET NOT NULL;
---INSERT INTO gtest21b (a) VALUES (1);  -- ok
---INSERT INTO gtest21b (a) VALUES (0);  -- violates constraint
+INSERT INTO gtest21b (a) VALUES (1);  -- ok
+INSERT INTO gtest21b (a) VALUES (2), (0);  -- violates constraint
+INSERT INTO gtest21b (a) VALUES (NULL);  -- error
 ALTER TABLE gtest21b ALTER COLUMN b DROP NOT NULL;
---INSERT INTO gtest21b (a) VALUES (0);  -- ok now
+INSERT INTO gtest21b (a) VALUES (0);  -- ok now
+
+--not null constraint with partitioned table case.
+CREATE TABLE gtestnn_parent(f1 int, f2 bigint, f3 bigint GENERATED ALWAYS AS (nullif(f1, 1) + nullif(f2, 10)) VIRTUAL not null) PARTITION BY RANGE (f1);
+CREATE TABLE gtestnn_child PARTITION OF gtestnn_parent FOR VALUES FROM (1) TO (5);
+CREATE TABLE gtestnn_childdef PARTITION OF gtestnn_parent default;
+--check the error message.
+INSERT INTO gtestnn_parent VALUES (2, 2, default), (3, 5, default), (14, 12, default);
+INSERT INTO gtestnn_parent VALUES (1, 2, default); --error
+INSERT INTO gtestnn_parent VALUES (2, 10, default); --error
+ALTER TABLE gtestnn_parent ALTER COLUMN f3 SET EXPRESSION AS (nullif(f1, 2) + nullif(f2, 11)); --error
+INSERT INTO gtestnn_parent VALUES (10, 11, default); --error
+select * from gtestnn_parent;
+--test ALTER TABLE ADD COLUMN.
+ALTER TABLE gtestnn_parent ADD COLUMN c int NOT NULL GENERATED ALWAYS AS (nullif(f1, 14) + nullif(f2, 10)) VIRTUAL; --error
+ALTER TABLE gtestnn_parent ADD COLUMN c int NOT NULL GENERATED ALWAYS AS (nullif(f1, 13) + nullif(f2, 5)) VIRTUAL; --error
+ALTER TABLE gtestnn_parent ADD COLUMN c int NOT NULL GENERATED ALWAYS AS (nullif(f1, 4) + nullif(f2, 6)) VIRTUAL; --ok
 
 -- index constraints
 CREATE TABLE gtest22a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a / 2) VIRTUAL UNIQUE);
@@ -524,13 +547,20 @@ CREATE TABLE gtest27 (
     b int,
     x int GENERATED ALWAYS AS ((a + b) * 2) VIRTUAL
 );
-INSERT INTO gtest27 (a, b) VALUES (3, 7), (4, 11);
+INSERT INTO gtest27 (a, b) VALUES (3, 7), (4, 11), (NULL, NULL);
 ALTER TABLE gtest27 ALTER COLUMN a TYPE text;  -- error
 ALTER TABLE gtest27 ALTER COLUMN x TYPE numeric;
 \d gtest27
 SELECT * FROM gtest27;
 ALTER TABLE gtest27 ALTER COLUMN x TYPE boolean USING x <> 0;  -- error
 ALTER TABLE gtest27 ALTER COLUMN x DROP DEFAULT;  -- error
+--not null error violation
+ALTER TABLE gtest27
+  DROP COLUMN x,
+  ALTER COLUMN a TYPE bigint,
+  ALTER COLUMN b TYPE bigint,
+  ADD COLUMN x bigint GENERATED ALWAYS AS ((a + b) * 2) VIRTUAL NOT NULL;
+DELETE FROM gtest27 WHERE a IS NULL AND b IS NULL;
 -- It's possible to alter the column types this way:
 ALTER TABLE gtest27
   DROP COLUMN x,
-- 
2.34.1

From 8b99e21fda58a0189547139b4aa2ba9f87eabbb7 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Thu, 20 Mar 2025 12:06:41 +0800
Subject: [PATCH v5 2/2] minor change ATRewriteTable

make notnull_attrs and notnull_virtual_attrs both are 1 based.
make sure TupleDescAttr need minus 1.
---
 src/backend/commands/tablecmds.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0751c0d627c..04806c068df 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6194,7 +6194,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
 			if (attr->attnotnull && !attr->attisdropped)
 			{
 				if (attr->attgenerated != ATTRIBUTE_GENERATED_VIRTUAL)
-					notnull_attrs = lappend_int(notnull_attrs, i);
+					notnull_attrs = lappend_int(notnull_attrs, attr->attnum);
 				else
 					notnull_virtual_attrs = lappend_int(notnull_virtual_attrs,
 														attr->attnum);
@@ -6405,20 +6405,18 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
 			/* Now check any constraints on the possibly-changed tuple */
 			econtext->ecxt_scantuple = insertslot;
 
-			foreach(l, notnull_attrs)
+			foreach_int(attn, notnull_attrs)
 			{
-				int			attn = lfirst_int(l);
-
-				if (slot_attisnull(insertslot, attn + 1))
+				if (slot_attisnull(insertslot, attn))
 				{
-					Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn);
+					Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn - 1);
 
 					ereport(ERROR,
 							(errcode(ERRCODE_NOT_NULL_VIOLATION),
 							 errmsg("column \"%s\" of relation \"%s\" contains null values",
 									NameStr(attr->attname),
 									RelationGetRelationName(oldrel)),
-							 errtablecol(oldrel, attn + 1)));
+							 errtablecol(oldrel, attn)));
 				}
 			}
 
-- 
2.34.1

Reply via email to