From 0985d7a779287feaa1a382726e566418bc56d381 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Sat, 8 Mar 2025 18:08:51 +0800
Subject: [PATCH v3 3/3] domain over virtual generated column.

domain don't have constraints works just be fine.
domain constraint check is happend within ExecComputeGenerated.
we compute the virtual generated column in ExecComputeGenerated and discarded the value.

domain_with_constaint can be element type of range, multirange, array, composite.
to be bullet proof, if this column type is not type of TYPTYPE_BASE or it's a array type,
then we do actlly compute the generated expression.
This can have negative performance for INSERT, if vertain virtual column data types requires computation.

the followig two query shows in pg_catalog, most of the system type is TYPTYPE_BASE.
so i guess this compromise is fine.

select count(*),typtype from pg_type where typnamespace = 11 group by 2;
select typname, typrelid, pc.reltype, pc.oid, pt.oid
from pg_type pt join pg_class pc on pc.oid = pt.typrelid
where pt.typnamespace = 11 and pt.typtype = 'c' and pc.reltype = 0;

ALTER DOMAIN ADD CONSTRAINT variant is supported.
DOMAIN with default values are supported. but virtual generated column already have
generated expression, so domain default expression doesn't matter.

ALTER TABLE ADD VIRTUAL GENERATED COLUMN.
need table rewrite to vertify new column value satisfied the generation expression.
this can be reduced to table scan in some cases.

TODO: deal with domain with volatile check constraints.
TODO: only table rewrites in somes cases.

discussion: https://postgr.es/m/CACJufxHArQysbDkWFmvK+D1TPHQWWTxWN15cMuUaTYX3xhQXgg@mail.gmail.com
---
 src/backend/catalog/heap.c                    |  11 -
 src/backend/commands/copyfrom.c               |   5 +-
 src/backend/commands/tablecmds.c              |   6 +-
 src/backend/commands/typecmds.c               | 210 +++++++++++++++---
 src/backend/executor/nodeModifyTable.c        |  84 +++++--
 .../regress/expected/generated_virtual.out    |  67 +++++-
 src/test/regress/sql/generated_virtual.sql    |  49 +++-
 7 files changed, 352 insertions(+), 80 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index b807ab66668..39e2ac38141 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -583,17 +583,6 @@ CheckAttributeType(const char *attname,
 	}
 	else if (att_typtype == TYPTYPE_DOMAIN)
 	{
-		/*
-		 * Prevent virtual generated columns from having a domain type.  We
-		 * would have to enforce domain constraints when columns underlying
-		 * the generated column change.  This could possibly be implemented,
-		 * but it's not.
-		 */
-		if (flags & CHKATYPE_IS_VIRTUAL)
-			ereport(ERROR,
-					errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					errmsg("virtual generated column \"%s\" cannot have a domain type", attname));
-
 		/*
 		 * If it's a domain, recurse to check its base type.
 		 */
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 7f1078dfe39..b8c984e1ec2 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1343,9 +1343,10 @@ CopyFrom(CopyFromState cstate)
 			}
 			else
 			{
-				/* Compute stored generated columns */
+				/* Compute generated columns */
 				if (resultRelInfo->ri_RelationDesc->rd_att->constr &&
-					resultRelInfo->ri_RelationDesc->rd_att->constr->has_generated_stored)
+					(resultRelInfo->ri_RelationDesc->rd_att->constr->has_generated_stored ||
+					 resultRelInfo->ri_RelationDesc->rd_att->constr->has_generated_virtual))
 					ExecComputeGenerated(resultRelInfo, estate, myslot,
 										 CMD_INSERT);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 41efda3d338..55411d3d5e1 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7510,10 +7510,14 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 				/*
 				 * Failed to use missing mode.  We have to do a table rewrite
 				 * to install the value --- unless it's a virtual generated
-				 * column.
+				 * column. However if virtual generated column is domain with
+				 * constraints then we have to table rewrite to evaulate the
+				 * generation expression. (TODO, in some case, we don't need rewrite)
 				 */
 				if (colDef->generated != ATTRIBUTE_GENERATED_VIRTUAL)
 					tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
+				else if (has_domain_constraints)
+					tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
 			}
 		}
 
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 3cb3ca1cca1..7ef84700619 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -65,6 +65,7 @@
 #include "parser/parse_expr.h"
 #include "parser/parse_func.h"
 #include "parser/parse_type.h"
+#include "rewrite/rewriteHandler.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
 #include "utils/inval.h"
@@ -144,6 +145,7 @@ static void AlterTypeRecurse(Oid typeOid, bool isImplicitArray,
 							 AlterTypeRecurseParams *atparams);
 
 
+static bool ValidateDomainNeedGeneratedExprs(RelToCheck *rtc);
 /*
  * DefineType
  *		Registers a new base type.
@@ -3112,6 +3114,28 @@ AlterDomainValidateConstraint(List *names, const char *constrName)
 	return address;
 }
 
+static bool
+ValidateDomainNeedGeneratedExprs(RelToCheck *rtc)
+{
+	Relation	testrel = rtc->rel;
+	TupleDesc	tupdesc = RelationGetDescr(testrel);
+
+	if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
+	{
+		int			i;
+		int			attnum;
+		Form_pg_attribute attr;
+		for (i = 0; i < rtc->natts; i++)
+		{
+			attnum = rtc->atts[i];
+			attr = TupleDescAttr(tupdesc, attnum - 1);
+			if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+				return true;
+		}
+	}
+	return false;
+}
+
 /*
  * Verify that all columns currently using the domain are not null.
  */
@@ -3121,6 +3145,15 @@ validateDomainNotNullConstraint(Oid domainoid)
 	List	   *rels;
 	ListCell   *rt;
 
+	/*
+	 * for domain not null constraint over virtual generated column
+	 * it maybe need it in below.
+	*/
+	EState	   *estate;
+	ExprContext *econtext;
+	estate = CreateExecutorState();
+	econtext = GetPerTupleExprContext(estate);
+
 	/* Fetch relation list with attributes based on this domain */
 	/* ShareLock is sufficient to prevent concurrent data changes */
 
@@ -3134,6 +3167,39 @@ validateDomainNotNullConstraint(Oid domainoid)
 		TupleTableSlot *slot;
 		TableScanDesc scan;
 		Snapshot	snapshot;
+		Form_pg_attribute attr;
+		ExprState **ri_GeneratedExprs = NULL;
+		bool		need_GeneratedExprs = false;
+		int			attnum;
+
+		/* cacahe generated columns handling */
+		need_GeneratedExprs = ValidateDomainNeedGeneratedExprs(rtc);
+		if (need_GeneratedExprs)
+		{
+			ri_GeneratedExprs = (ExprState **) palloc0(tupdesc->natts * sizeof(ExprState *));
+
+			for (int i = 0; i < rtc->natts; i++)
+			{
+				attnum = rtc->atts[i];
+				attr = TupleDescAttr(tupdesc, attnum - 1);
+
+				if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+				{
+					NullTest   *nnulltest;
+					Expr	   *expr;
+
+					nnulltest = makeNode(NullTest);
+					nnulltest->arg = (Expr *) build_column_default(testrel, attnum);
+					nnulltest->nulltesttype = IS_NOT_NULL;
+					nnulltest->argisrow = false;
+					nnulltest->location = -1;
+
+					expr = (Expr *) nnulltest;
+					ri_GeneratedExprs[attnum - 1] = ExecPrepareExpr(expr, estate);
+				}
+			}
+		}
+
 
 		/* Scan all tuples in this relation */
 		snapshot = RegisterSnapshot(GetLatestSnapshot());
@@ -3146,8 +3212,8 @@ validateDomainNotNullConstraint(Oid domainoid)
 			/* Test attributes that are of the domain */
 			for (i = 0; i < rtc->natts; i++)
 			{
-				int			attnum = rtc->atts[i];
-				Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1);
+				attnum = rtc->atts[i];
+				attr = TupleDescAttr(tupdesc, attnum - 1);
 
 				if (slot_attisnull(slot, attnum))
 				{
@@ -3158,14 +3224,30 @@ validateDomainNotNullConstraint(Oid domainoid)
 					 * only executes in an ALTER DOMAIN command, the client
 					 * should already know which domain is in question.
 					 */
-					ereport(ERROR,
-							(errcode(ERRCODE_NOT_NULL_VIOLATION),
-							 errmsg("column \"%s\" of table \"%s\" contains null values",
-									NameStr(attr->attname),
-									RelationGetRelationName(testrel)),
-							 errtablecol(testrel, attnum)));
+					if (attr->attgenerated != ATTRIBUTE_GENERATED_VIRTUAL)
+						ereport(ERROR,
+								(errcode(ERRCODE_NOT_NULL_VIOLATION),
+								errmsg("column \"%s\" of table \"%s\" contains null values",
+										NameStr(attr->attname),
+										RelationGetRelationName(testrel)),
+								errtablecol(testrel, attnum)));
+					else
+					{
+						Assert(need_GeneratedExprs);
+						econtext->ecxt_scantuple = slot;
+						Assert(ri_GeneratedExprs[attnum - 1] != NULL);
+						if (!ExecCheck(ri_GeneratedExprs[attnum - 1] , econtext))
+							ereport(ERROR,
+									errcode(ERRCODE_NOT_NULL_VIOLATION),
+									errmsg("column \"%s\" of table \"%s\" contains null values",
+											NameStr(attr->attname),
+											RelationGetRelationName(testrel)),
+									errtablecol(testrel, attnum));
+					}
 				}
 			}
+
+			ResetExprContext(econtext);
 		}
 		ExecDropSingleTupleTableSlot(slot);
 		table_endscan(scan);
@@ -3174,6 +3256,8 @@ validateDomainNotNullConstraint(Oid domainoid)
 		/* Close each rel after processing, but keep lock */
 		table_close(testrel, NoLock);
 	}
+
+	FreeExecutorState(estate);
 }
 
 /*
@@ -3210,6 +3294,32 @@ validateDomainCheckConstraint(Oid domainoid, const char *ccbin)
 		TupleTableSlot *slot;
 		TableScanDesc scan;
 		Snapshot	snapshot;
+		ExprState **ri_GeneratedExprs = NULL;
+		bool		need_GeneratedExprs = false;
+		Form_pg_attribute attr;
+		int			attnum;
+
+		/* cacahe generated columns handling */
+		need_GeneratedExprs = ValidateDomainNeedGeneratedExprs(rtc);
+
+		if (need_GeneratedExprs)
+		{
+			ri_GeneratedExprs = (ExprState **) palloc0(tupdesc->natts * sizeof(ExprState *));
+			for (int i = 0; i < rtc->natts; i++)
+			{
+				Expr	   *gen_defexpr;
+
+				attnum = rtc->atts[i];
+				attr = TupleDescAttr(tupdesc, attnum - 1);
+
+				/* Fetch the GENERATED AS expression tree */
+				gen_defexpr = (Expr *) build_column_default(testrel, attnum);
+				if (gen_defexpr == NULL)
+					elog(ERROR, "no generation expression found for column number %d of table \"%s\"",
+						attnum, RelationGetRelationName(testrel));
+				ri_GeneratedExprs[attnum - 1] = ExecPrepareExpr(gen_defexpr, estate);
+			}
+		}
 
 		/* Scan all tuples in this relation */
 		snapshot = RegisterSnapshot(GetLatestSnapshot());
@@ -3222,37 +3332,75 @@ validateDomainCheckConstraint(Oid domainoid, const char *ccbin)
 			/* Test attributes that are of the domain */
 			for (i = 0; i < rtc->natts; i++)
 			{
-				int			attnum = rtc->atts[i];
 				Datum		d;
 				bool		isNull;
 				Datum		conResult;
-				Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1);
 
-				d = slot_getattr(slot, attnum, &isNull);
+				attnum = rtc->atts[i];
+				attr = TupleDescAttr(tupdesc, attnum - 1);
 
-				econtext->domainValue_datum = d;
-				econtext->domainValue_isNull = isNull;
-
-				conResult = ExecEvalExprSwitchContext(exprstate,
-													  econtext,
-													  &isNull);
-
-				if (!isNull && !DatumGetBool(conResult))
+				/*
+				 * we first actually compute the virtual generated column value
+				 * then do the CoerceToDomainValue check.
+				 */
+				if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
 				{
+					Datum		ret;
+					bool		isnull;
+
+					Assert(need_GeneratedExprs);
+					econtext = GetPerTupleExprContext(estate);
+					econtext->ecxt_scantuple = slot;
+
+					ret = ExecEvalExprSwitchContext(ri_GeneratedExprs[attnum - 1],
+													econtext,
+													&isnull);
 					/*
-					 * In principle the auxiliary information for this error
-					 * should be errdomainconstraint(), but errtablecol()
-					 * seems considerably more useful in practice.  Since this
-					 * code only executes in an ALTER DOMAIN command, the
-					 * client should already know which domain is in question,
-					 * and which constraint too.
+					 * we probally don't need datumCopy to copy ret, since here
+					 * we just check the value.
 					 */
-					ereport(ERROR,
-							(errcode(ERRCODE_CHECK_VIOLATION),
-							 errmsg("column \"%s\" of table \"%s\" contains values that violate the new constraint",
-									NameStr(attr->attname),
-									RelationGetRelationName(testrel)),
-							 errtablecol(testrel, attnum)));
+					econtext->domainValue_datum = ret;
+					econtext->domainValue_isNull = isnull;
+					econtext->ecxt_scantuple = NULL;
+
+					conResult = ExecEvalExprSwitchContext(exprstate,
+														  econtext,
+														  &isNull);
+					if (!isNull && !DatumGetBool(conResult))
+						ereport(ERROR,
+								errcode(ERRCODE_CHECK_VIOLATION),
+								errmsg("column \"%s\" of table \"%s\" contains values that violate the new constraint",
+										NameStr(attr->attname), RelationGetRelationName(testrel)),
+								errtablecol(testrel, attnum));
+				}
+				else
+				{
+					d = slot_getattr(slot, attnum, &isNull);
+
+					econtext->domainValue_datum = d;
+					econtext->domainValue_isNull = isNull;
+
+					conResult = ExecEvalExprSwitchContext(exprstate,
+														  econtext,
+														  &isNull);
+
+					if (!isNull && !DatumGetBool(conResult))
+					{
+						/*
+						* In principle the auxiliary information for this error
+						* should be errdomainconstraint(), but errtablecol()
+						* seems considerably more useful in practice.  Since this
+						* code only executes in an ALTER DOMAIN command, the
+						* client should already know which domain is in question,
+						* and which constraint too.
+						*/
+						ereport(ERROR,
+								(errcode(ERRCODE_CHECK_VIOLATION),
+								errmsg("column \"%s\" of table \"%s\" contains values that violate the new constraint",
+										NameStr(attr->attname),
+										RelationGetRelationName(testrel)),
+								errtablecol(testrel, attnum)));
+					}
 				}
 			}
 
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 53755ccdbad..e83e57bea0a 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -67,6 +67,7 @@
 #include "storage/lmgr.h"
 #include "utils/builtins.h"
 #include "utils/datum.h"
+#include "utils/lsyscache.h"
 #include "utils/rel.h"
 #include "utils/snapmgr.h"
 
@@ -395,7 +396,7 @@ 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 used for stored and virtual generated columns.
  *
  * If cmdType == CMD_UPDATE, the ri_extraUpdatedCols field is filled too.
  * This is used by both stored and virtual generated columns.
@@ -477,6 +478,33 @@ ExecInitGenerated(ResultRelInfo *resultRelInfo,
 				ri_GeneratedExprs[i] = ExecPrepareExpr(expr, estate);
 				ri_NumGeneratedNeeded++;
 			}
+			else
+			{
+				/*
+				* Virtual generated columns only need to be computed when the
+				* type is domain_with_constraint.  However,
+				* domain_with_constraint can be the element type of a range, the
+				* element type of a composite, or the element type of an array.
+				*
+				* To determine whether the true base element type is
+				* domain_with_constraint, multiple lookup_type_cache calls seems
+				* doable. However, this would add a lot of code.
+				* So, simplified it: only if the type is TYPTYPE_BASE and it's
+				* not an array type, computation is not needed.
+				*/
+				Oid		typelem	= InvalidOid;
+				Oid		atttypid = TupleDescAttr(tupdesc, i)->atttypid;
+				char	att_typtype = get_typtype(atttypid);
+
+				if (att_typtype == TYPTYPE_BASE)
+					typelem = get_element_type(atttypid);
+
+				if (att_typtype != TYPTYPE_BASE || OidIsValid(typelem))
+				{
+					ri_GeneratedExprs[i] = ExecPrepareExpr(expr, estate);
+					ri_NumGeneratedNeeded++;
+				}
+			}
 
 			/* If UPDATE, mark column in resultRelInfo->ri_extraUpdatedCols */
 			if (cmdtype == CMD_UPDATE)
@@ -518,7 +546,9 @@ ExecInitGenerated(ResultRelInfo *resultRelInfo,
 
 /*
  * Compute generated columns for a tuple.
- * we might support virtual generated column in future, currently not.
+ * Generally, we don't need compute virtual generated column, except for column
+ * type is domain with constraint. Since virtual generated column don't have
+ * storage, we don't need stored the computed value.
  */
 void
 ExecComputeGenerated(ResultRelInfo *resultRelInfo, EState *estate,
@@ -532,9 +562,11 @@ ExecComputeGenerated(ResultRelInfo *resultRelInfo, EState *estate,
 	MemoryContext oldContext;
 	Datum	   *values;
 	bool	   *nulls;
+	bool	   need_compute = false;
 
 	/* 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
@@ -553,7 +585,8 @@ ExecComputeGenerated(ResultRelInfo *resultRelInfo, EState *estate,
 		if (resultRelInfo->ri_GeneratedExprsI == NULL)
 			ExecInitGenerated(resultRelInfo, estate, cmdtype);
 		/* Early exit is impossible given the prior Assert */
-		Assert(resultRelInfo->ri_NumGeneratedNeededI > 0);
+		if (resultRelInfo->ri_GeneratedExprsI != NULL)
+			Assert(resultRelInfo->ri_NumGeneratedNeededI > 0);
 		ri_GeneratedExprs = resultRelInfo->ri_GeneratedExprsI;
 	}
 
@@ -565,30 +598,40 @@ ExecComputeGenerated(ResultRelInfo *resultRelInfo, EState *estate,
 	slot_getallattrs(slot);
 	memcpy(nulls, slot->tts_isnull, sizeof(*nulls) * natts);
 
+	need_compute = (resultRelInfo->ri_NumGeneratedNeededI > 0 ||
+					resultRelInfo->ri_NumGeneratedNeededU > 0);
+
 	for (int i = 0; i < natts; i++)
 	{
 		CompactAttribute *attr = TupleDescCompactAttr(tupdesc, i);
 
-		if (ri_GeneratedExprs[i])
+		if (need_compute && ri_GeneratedExprs[i] )
 		{
 			Datum		val;
 			bool		isnull;
 
-			Assert(TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_STORED);
-
 			econtext->ecxt_scantuple = slot;
 
 			val = ExecEvalExpr(ri_GeneratedExprs[i], econtext, &isnull);
 
-			/*
-			 * We must make a copy of val as we have no guarantees about where
-			 * memory for a pass-by-reference Datum is located.
-			 */
-			if (!isnull)
-				val = datumCopy(val, attr->attbyval, attr->attlen);
+			if (TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_STORED)
+			{
+				/*
+				 * We must make a copy of val as we have no guarantees about where
+				 * memory for a pass-by-reference Datum is located.
+				*/
+				if (!isnull)
+					val = datumCopy(val, attr->attbyval, attr->attlen);
 
-			values[i] = val;
-			nulls[i] = isnull;
+				values[i] = val;
+				nulls[i] = isnull;
+			}
+			else
+			{
+				Assert(TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL);
+				values[i] = (Datum) 0;
+				nulls[i] = true;
+			}
 		}
 		else
 		{
@@ -910,7 +953,8 @@ ExecInsert(ModifyTableContext *context,
 		 * Compute stored generated columns
 		 */
 		if (resultRelationDesc->rd_att->constr &&
-			resultRelationDesc->rd_att->constr->has_generated_stored)
+			(resultRelationDesc->rd_att->constr->has_generated_stored ||
+			 resultRelationDesc->rd_att->constr->has_generated_virtual))
 			ExecComputeGenerated(resultRelInfo, estate, slot,
 								 CMD_INSERT);
 
@@ -1037,7 +1081,8 @@ ExecInsert(ModifyTableContext *context,
 		 * Compute stored generated columns
 		 */
 		if (resultRelationDesc->rd_att->constr &&
-			resultRelationDesc->rd_att->constr->has_generated_stored)
+			(resultRelationDesc->rd_att->constr->has_generated_stored ||
+			 resultRelationDesc->rd_att->constr->has_generated_virtual))
 			ExecComputeGenerated(resultRelInfo, estate, slot,
 								 CMD_INSERT);
 
@@ -2122,10 +2167,11 @@ ExecUpdatePrepareSlot(ResultRelInfo *resultRelInfo,
 	slot->tts_tableOid = RelationGetRelid(resultRelationDesc);
 
 	/*
-	 * Compute stored generated columns
+	 * Compute generated columns
 	 */
 	if (resultRelationDesc->rd_att->constr &&
-		resultRelationDesc->rd_att->constr->has_generated_stored)
+		(resultRelationDesc->rd_att->constr->has_generated_stored ||
+		 resultRelationDesc->rd_att->constr->has_generated_virtual))
 		ExecComputeGenerated(resultRelInfo, estate, slot,
 							 CMD_UPDATE);
 }
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index 0eef3fcfff1..7c8bacf9232 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -785,16 +785,65 @@ ERROR:  relation "gtest23p" does not exist
 --INSERT INTO gtest23q VALUES (1, 2);  -- ok
 --INSERT INTO gtest23q VALUES (2, 5);  -- error
 -- domains
-CREATE DOMAIN gtestdomain1 AS int CHECK (VALUE < 10);
-CREATE TABLE gtest24 (a int PRIMARY KEY, b gtestdomain1 GENERATED ALWAYS AS (a * 2) VIRTUAL);
-ERROR:  virtual generated column "b" cannot have a domain type
---INSERT INTO gtest24 (a) VALUES (4);  -- ok
---INSERT INTO gtest24 (a) VALUES (6);  -- error
+CREATE DOMAIN gtestdomain1 AS int CHECK (VALUE < 10) DEFAULT 12;
+CREATE TABLE gtest24 (a int UNIQUE, b gtestdomain1 GENERATED ALWAYS AS (a * 2) VIRTUAL);
+INSERT INTO gtest24 (a, b) VALUES (4, default);  -- ok
+INSERT INTO gtest24 (a) VALUES (3), (NULL);  -- ok
+INSERT INTO gtest24 (a) VALUES (6);  -- error
+ERROR:  value for domain gtestdomain1 violates check constraint "gtestdomain1_check"
+UPDATE gtest24 SET a = 6;            -- error
+ERROR:  value for domain gtestdomain1 violates check constraint "gtestdomain1_check"
+COPY gtest24 FROM stdin;  --error
+ERROR:  value for domain gtestdomain1 violates check constraint "gtestdomain1_check"
+CONTEXT:  COPY gtest24, line 1: "6"
+SELECT * FROM gtest24;
+ a | b 
+---+---
+ 4 | 8
+ 3 | 6
+   |  
+(3 rows)
+
+--ALTER TABLE ADD COLUM variant.
+ALTER TABLE gtest24 ADD COLUMN c gtestdomain1 GENERATED ALWAYS AS (a * 2) virtual not null; --error
+ERROR:  column "c" of relation "gtest24" contains null values
+ALTER TABLE gtest24 ADD COLUMN c gtestdomain1 GENERATED ALWAYS AS (a * 3) virtual check (c < 10); --error
+ERROR:  value for domain gtestdomain1 violates check constraint "gtestdomain1_check"
+ALTER TABLE gtest24 ADD COLUMN c gtestdomain1 GENERATED ALWAYS AS (a * 4) virtual; --table rewrite then error.
+ERROR:  value for domain gtestdomain1 violates check constraint "gtestdomain1_check"
+ALTER TABLE gtest24 ADD COLUMN c gtestdomain1 GENERATED ALWAYS AS (a * 2) virtual; --ok
+--alter domain add constraint variant.
+ALTER DOMAIN gtestdomain1 ADD CHECK (value IS NULL);  --error
+ERROR:  column "b" of table "gtest24" contains values that violate the new constraint
+ALTER DOMAIN gtestdomain1 ADD CHECK (value IS NOT NULL); --error
+ERROR:  column "b" of table "gtest24" contains values that violate the new constraint
+ALTER DOMAIN gtestdomain1 ADD NOT NULL; --error
+ERROR:  column "b" of table "gtest24" contains null values
+DELETE FROM gtest24 WHERE a is NULL;
+ALTER DOMAIN gtestdomain1 ADD NOT NULL; --ok
+ALTER DOMAIN gtestdomain1 ADD CHECK (value IS NOT NULL);  --ok
+ALTER DOMAIN gtestdomain1 ADD CHECK (VALUE < 7); --error
+ERROR:  column "b" of table "gtest24" contains values that violate the new constraint
+ALTER DOMAIN gtestdomain1 ADD CHECK (VALUE < 9); --ok
+INSERT INTO gtest24 (a) VALUES (NULL);  -- error
+ERROR:  domain gtestdomain1 does not allow null values
 CREATE TYPE gtestdomain1range AS range (subtype = gtestdomain1);
-CREATE TABLE gtest24r (a int PRIMARY KEY, b gtestdomain1range GENERATED ALWAYS AS (gtestdomain1range(a, a + 5)) VIRTUAL);
-ERROR:  virtual generated column "b" cannot have a domain type
---INSERT INTO gtest24r (a) VALUES (4);  -- ok
---INSERT INTO gtest24r (a) VALUES (6);  -- error
+CREATE TABLE gtest24r (a int PRIMARY KEY, b gtestdomain1range GENERATED ALWAYS AS (gtestdomain1range(a, a + 4)) VIRTUAL);
+INSERT INTO gtest24r (a) VALUES (4);  -- ok
+INSERT INTO gtest24r (a) VALUES (6);  -- error
+ERROR:  value for domain gtestdomain1 violates check constraint "gtestdomain1_check"
+INSERT INTO gtest24r (a) VALUES (5);  -- error
+ERROR:  value for domain gtestdomain1 violates check constraint "gtestdomain1_check2"
+CREATE TABLE gtest24v (
+  a jsonb,
+  b gtestdomain1[] GENERATED ALWAYS AS (JSON_QUERY(a, '$.a' returning gtestdomain1[] error on error)) VIRTUAL,
+  c gtestdomain1[] GENERATED ALWAYS AS (JSON_QUERY(a, '$.b' returning gtestdomain1[])) VIRTUAL not null);
+insert into gtest24v select jsonb '{"a":[6,10], "b":[6,2]}'; --error
+ERROR:  value for domain gtestdomain1 violates check constraint "gtestdomain1_check"
+insert into gtest24v select jsonb '{"a":[6,-1], "b":[6,10]}'; --error
+ERROR:  null value in column "c" of relation "gtest24v" violates not-null constraint
+DETAIL:  Failing row contains ({"a": [6, -1], "b": [6, 10]}, virtual, virtual).
+insert into gtest24v select jsonb '{"a":[6,-1], "b":[6,2]}'; --ok
 -- typed tables (currently not supported)
 CREATE TYPE gtest_type AS (f1 integer, f2 text, f3 bigint);
 CREATE TABLE gtest28 OF gtest_type (f1 WITH OPTIONS GENERATED ALWAYS AS (f2 *2) VIRTUAL);
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index 485e2a317fe..81f03cc4a92 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -439,14 +439,49 @@ CREATE TABLE gtest23q (a int PRIMARY KEY, b int REFERENCES gtest23p (y));
 --INSERT INTO gtest23q VALUES (2, 5);  -- error
 
 -- domains
-CREATE DOMAIN gtestdomain1 AS int CHECK (VALUE < 10);
-CREATE TABLE gtest24 (a int PRIMARY KEY, b gtestdomain1 GENERATED ALWAYS AS (a * 2) VIRTUAL);
---INSERT INTO gtest24 (a) VALUES (4);  -- ok
---INSERT INTO gtest24 (a) VALUES (6);  -- error
+CREATE DOMAIN gtestdomain1 AS int CHECK (VALUE < 10) DEFAULT 12;
+CREATE TABLE gtest24 (a int UNIQUE, b gtestdomain1 GENERATED ALWAYS AS (a * 2) VIRTUAL);
+INSERT INTO gtest24 (a, b) VALUES (4, default);  -- ok
+INSERT INTO gtest24 (a) VALUES (3), (NULL);  -- ok
+INSERT INTO gtest24 (a) VALUES (6);  -- error
+UPDATE gtest24 SET a = 6;            -- error
+COPY gtest24 FROM stdin;  --error
+6
+\.
+
+SELECT * FROM gtest24;
+
+--ALTER TABLE ADD COLUM variant.
+ALTER TABLE gtest24 ADD COLUMN c gtestdomain1 GENERATED ALWAYS AS (a * 2) virtual not null; --error
+ALTER TABLE gtest24 ADD COLUMN c gtestdomain1 GENERATED ALWAYS AS (a * 3) virtual check (c < 10); --error
+ALTER TABLE gtest24 ADD COLUMN c gtestdomain1 GENERATED ALWAYS AS (a * 4) virtual; --table rewrite then error.
+ALTER TABLE gtest24 ADD COLUMN c gtestdomain1 GENERATED ALWAYS AS (a * 2) virtual; --ok
+
+--alter domain add constraint variant.
+ALTER DOMAIN gtestdomain1 ADD CHECK (value IS NULL);  --error
+ALTER DOMAIN gtestdomain1 ADD CHECK (value IS NOT NULL); --error
+ALTER DOMAIN gtestdomain1 ADD NOT NULL; --error
+DELETE FROM gtest24 WHERE a is NULL;
+ALTER DOMAIN gtestdomain1 ADD NOT NULL; --ok
+ALTER DOMAIN gtestdomain1 ADD CHECK (value IS NOT NULL);  --ok
+ALTER DOMAIN gtestdomain1 ADD CHECK (VALUE < 7); --error
+ALTER DOMAIN gtestdomain1 ADD CHECK (VALUE < 9); --ok
+
+INSERT INTO gtest24 (a) VALUES (NULL);  -- error
+
 CREATE TYPE gtestdomain1range AS range (subtype = gtestdomain1);
-CREATE TABLE gtest24r (a int PRIMARY KEY, b gtestdomain1range GENERATED ALWAYS AS (gtestdomain1range(a, a + 5)) VIRTUAL);
---INSERT INTO gtest24r (a) VALUES (4);  -- ok
---INSERT INTO gtest24r (a) VALUES (6);  -- error
+CREATE TABLE gtest24r (a int PRIMARY KEY, b gtestdomain1range GENERATED ALWAYS AS (gtestdomain1range(a, a + 4)) VIRTUAL);
+INSERT INTO gtest24r (a) VALUES (4);  -- ok
+INSERT INTO gtest24r (a) VALUES (6);  -- error
+INSERT INTO gtest24r (a) VALUES (5);  -- error
+
+CREATE TABLE gtest24v (
+  a jsonb,
+  b gtestdomain1[] GENERATED ALWAYS AS (JSON_QUERY(a, '$.a' returning gtestdomain1[] error on error)) VIRTUAL,
+  c gtestdomain1[] GENERATED ALWAYS AS (JSON_QUERY(a, '$.b' returning gtestdomain1[])) VIRTUAL not null);
+insert into gtest24v select jsonb '{"a":[6,10], "b":[6,2]}'; --error
+insert into gtest24v select jsonb '{"a":[6,-1], "b":[6,10]}'; --error
+insert into gtest24v select jsonb '{"a":[6,-1], "b":[6,2]}'; --ok
 
 -- typed tables (currently not supported)
 CREATE TYPE gtest_type AS (f1 integer, f2 text, f3 bigint);
-- 
2.34.1

