On Wed, Feb 26, 2025 at 3:01 PM ego alter <xunengz...@gmail.com> wrote:
>
> Hi, I’ve had a chance to review the patch. As I am still getting familiar
> with executor part, questions and feedbacks could be relatively trivial.
>
> There are two minor issues i want to discuss:
> 1. The way of caching constraint-checking expr for virtual generated not null
> The existing array for caching constraint-checking expr is
> /* array of constraint-checking expr states */
> ExprState **ri_ConstraintExprs;
>
> the proposed changes for virtual generated not null in patch
> + /* array of virtual generated not null constraint-checking expr states */
> + ExprState **ri_VirGeneratedConstraintExprs;
> /*
> Could you explain the rationale behind adding this new field instead of
> reusing ri_ConstraintExprs? The comment suggests it’s used specifically for
> not null constraint-checking, but could it be extended to handle other
> constraints in the future as well? I assume one benefit is that it separates
> the handling of normal constraints from virtual ones, but I'd like to
> appreciate more context on the decision.
>
it's a cache mechanism, just like ResultRelInfo.ri_ConstraintExprs
you can see comments in ExecRelCheck
/*
* If first time through for this result relation, build expression
* nodetrees for rel's constraint expressions. Keep them in the per-query
* memory context so they'll survive throughout the query.
*/
for example:
create table t(a int, check (a > 3));
insert into t values (4),(3);
we need to call ExecRelCheck for values 4 and 3.
The second time ExecRelCheck called for values 3, if
ri_ConstraintExprs is not null then
we didn't need to call ExecPrepareExpr again for the same check
constraint expression.
+ ExprState **ri_VirGeneratedConstraintExprs;
is specifically only for virtual generated columns not null constraint
evaluation.
Anyway, I renamed it to ri_GeneratedNotNullExprs.
If you want to extend cache for other constraints in the future,
you can add it to struct ResultRelInfo.
Currently struct ResultRelInfo, we have ri_GeneratedExprsU,
ri_GeneratedExprsI for generated expressions;
ri_ConstraintExprs for check constraints.
> 2. The naming of variable gen_virtualnn.
> Gen_virtualnn looks confusing at first glance. Checkconstr seems to be more
> straightforward.
>
thanks. I changed it to exprstate.
new patch attached.
0001 not null for virtual generated columns, some refactoring happened
within ExecConstraints
to avoid duplicated error handling code.
0002 and 0003 is the domain for virtual generated columns. domain can
have not-null constraints,
this is built on top of 0001, so I guess posting here for review should be fine?
currently it works as intended. but add a virtual generated column
with domain_with_constraints
requires table rewrite. but some cases table scan should just be enough.
some case we do need table rewrite.
for example:
create domain d1 as int check(value > random(min=>11::int, max=>12));
create domain d2 as int check(value > 12);
create table t(a int);
insert into t select g from generate_series(1, 10) g;
alter table t add column b d1 generated always as (a+11) virtual; --we
do need table rewrite in phase 3.
alter table t add column c d2 generated always as (a + 12) virtual;
--we can only do table scan in phase 3.
From 5e1c83c18cd954abcbae4bd52ef9c77de1273f9c Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Thu, 6 Mar 2025 12:35:19 +0800
Subject: [PATCH v2 2/3] rename ExecComputeGenerated to ExecComputeGenerated
to support virtual generated column over domain type, we actually
need compute the virtual generated expression.
we did it at ExecComputeGenerated for stored, we can use it for virtual.
so do the rename.
discussion: https://postgr.es/m/cacjufxharqysbdkwfmvk+d1tphqwwtxwn15cmuuatyx3xhq...@mail.gmail.com
---
src/backend/commands/copyfrom.c | 4 ++--
src/backend/executor/execReplication.c | 8 ++++----
src/backend/executor/nodeModifyTable.c | 20 ++++++++++----------
src/include/executor/nodeModifyTable.h | 5 ++---
4 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index bcf66f0adf8..7f1078dfe39 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1346,8 +1346,8 @@ 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);
/*
* If the target is a plain table, check the constraints of
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 5cef54f00ed..fdc379e7a44 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -549,8 +549,8 @@ 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);
/* Check the constraints of the tuple */
if (rel->rd_att->constr)
@@ -646,8 +646,8 @@ 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);
/* Check the constraints of the tuple */
if (rel->rd_att->constr)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index b0fe50075ad..53755ccdbad 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -517,12 +517,12 @@ ExecInitGenerated(ResultRelInfo *resultRelInfo,
}
/*
- * Compute stored generated columns for a tuple
+ * Compute generated columns for a tuple.
+ * we might support virtual generated column in future, currently not.
*/
void
-ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
- EState *estate, TupleTableSlot *slot,
- CmdType cmdtype)
+ExecComputeGenerated(ResultRelInfo *resultRelInfo, EState *estate,
+ TupleTableSlot *slot, CmdType cmdtype)
{
Relation rel = resultRelInfo->ri_RelationDesc;
TupleDesc tupdesc = RelationGetDescr(rel);
@@ -911,8 +911,8 @@ 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);
/*
* If the FDW supports batching, and batching is requested, accumulate
@@ -1038,8 +1038,8 @@ 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);
/*
* Check any RLS WITH CHECK policies.
@@ -2126,8 +2126,8 @@ 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);
}
/*
diff --git a/src/include/executor/nodeModifyTable.h b/src/include/executor/nodeModifyTable.h
index bf3b592e28f..de374c46d3c 100644
--- a/src/include/executor/nodeModifyTable.h
+++ b/src/include/executor/nodeModifyTable.h
@@ -19,9 +19,8 @@ extern void ExecInitGenerated(ResultRelInfo *resultRelInfo,
EState *estate,
CmdType cmdtype);
-extern void ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
- EState *estate, TupleTableSlot *slot,
- CmdType cmdtype);
+extern void ExecComputeGenerated(ResultRelInfo *resultRelInfo, EState *estate,
+ TupleTableSlot *slot,CmdType cmdtype);
extern ModifyTableState *ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags);
extern void ExecEndModifyTable(ModifyTableState *node);
--
2.34.1
From a4a5343dfbac4130bbf6e3fb63992b8ac03591cd Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Thu, 6 Mar 2025 21:17:52 +0800
Subject: [PATCH v2 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+d1tphqwwtxwn15cmuuatyx3xhq...@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 | 198 +++++++++++++++---
src/backend/executor/nodeModifyTable.c | 84 ++++++--
.../regress/expected/generated_virtual.out | 67 +++++-
src/test/regress/sql/generated_virtual.sql | 49 ++++-
7 files changed, 345 insertions(+), 75 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 c89f1cb9936..d314cfb8f7d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7453,10 +7453,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..a247aa937f9 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;
+ 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,10 +3212,10 @@ 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))
+ if (slot_attisnull(slot, attnum) && attr->attgenerated != ATTRIBUTE_GENERATED_VIRTUAL)
{
/*
* In principle the auxiliary information for this error
@@ -3165,7 +3231,22 @@ validateDomainNotNullConstraint(Oid domainoid)
RelationGetRelationName(testrel)),
errtablecol(testrel, attnum)));
}
+
+ if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+ {
+ 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 +3255,8 @@ validateDomainNotNullConstraint(Oid domainoid)
/* Close each rel after processing, but keep lock */
table_close(testrel, NoLock);
}
+
+ FreeExecutorState(estate);
}
/*
@@ -3210,6 +3293,32 @@ validateDomainCheckConstraint(Oid domainoid, const char *ccbin)
TupleTableSlot *slot;
TableScanDesc scan;
Snapshot snapshot;
+ ExprState **ri_GeneratedExprs;
+ 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 +3331,74 @@ 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;
+
+ 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 3bd5130587c..c1528477cea 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -763,16 +763,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 f436e8f2313..0b3a981f225 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -429,14 +429,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
From ea676b927f0d74c1dde6237cd2f5b613af44053d Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Thu, 6 Mar 2025 12:01:31 +0800
Subject: [PATCH v2 1/3] not null for virtual generated column
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.
discussion: https://postgr.es/m/cacjufxharqysbdkwfmvk+d1tphqwwtxwn15cmuuatyx3xhq...@mail.gmail.com
---
src/backend/catalog/heap.c | 10 -
src/backend/commands/indexcmds.c | 10 +-
src/backend/commands/tablecmds.c | 8 -
src/backend/executor/execMain.c | 234 ++++++++++++++----
src/backend/parser/parse_utilcmd.c | 14 --
src/include/nodes/execnodes.h | 2 +
.../regress/expected/generated_virtual.out | 54 ++--
src/test/regress/sql/generated_virtual.sql | 31 ++-
8 files changed, 247 insertions(+), 116 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 32ff3ca9a28..e690487eb3e 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1126,10 +1126,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 key 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 59156a1c1f6..c89f1cb9936 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7824,14 +7824,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))
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 0493b7d5365..5f0b0cc3b8e 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"
@@ -1372,6 +1373,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;
@@ -1841,6 +1843,78 @@ ExecutePlan(QueryDesc *queryDesc,
}
+/*
+ * we warp "col IS NOT NULL" to a NullTest node. NullTest->arg is the virtual
+ * generated expression.
+ * A return value of -1 means virtual generated not null constraint satisfied,
+ * return value > 0 means not null violation happend in that attribute number.
+ *
+ * gen_virtual_cols is the attribute numbers of virtual generated column.
+*/
+static int
+ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
+ EState *estate, Bitmapset *gen_virtual_cols)
+{
+ Relation rel = resultRelInfo->ri_RelationDesc;
+ ExprContext *econtext;
+ MemoryContext oldContext;
+ int i = 0;
+ int attidx = -1;
+ int *attnums;
+ int cnt = bms_num_members(gen_virtual_cols);
+
+ attnums = (int *) palloc0(cnt * sizeof(int));
+ if (resultRelInfo->ri_GeneratedNotNullExprs == NULL)
+ {
+ oldContext = MemoryContextSwitchTo(estate->es_query_cxt);
+ resultRelInfo->ri_GeneratedNotNullExprs =
+ (ExprState **) palloc0(cnt * sizeof(ExprState *));
+
+ while ((attidx = bms_next_member(gen_virtual_cols, attidx)) >= 0)
+ {
+ Expr *expr;
+ NullTest *nnulltest;
+
+ nnulltest = makeNode(NullTest);
+ nnulltest->arg = (Expr *) build_generation_expression(rel, attidx);
+ nnulltest->nulltesttype = IS_NOT_NULL;
+ nnulltest->argisrow = false;
+ nnulltest->location = -1;
+
+ expr = (Expr *) nnulltest;
+ resultRelInfo->ri_GeneratedNotNullExprs[i] = ExecPrepareExpr(expr, estate);
+ attnums[i++] = attidx;
+ }
+ MemoryContextSwitchTo(oldContext);
+ }
+ else
+ {
+ while ((attidx = bms_next_member(gen_virtual_cols, attidx)) >= 0)
+ attnums[i++] = attidx;
+ }
+
+ /*
+ * 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 */
+ for (i = 0; i < cnt; i++)
+ {
+ ExprState *exprstate = resultRelInfo->ri_GeneratedNotNullExprs[i];
+ if (exprstate && !ExecCheck(exprstate, econtext))
+ return attnums[i];
+ }
+
+ /* -1 result means no error */
+ return -1;
+}
+
/*
* ExecRelCheck --- check that tuple meets constraints for result relation
*
@@ -2039,6 +2113,70 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
errtable(resultRelInfo->ri_RelationDesc)));
}
+/* not null constraint violation happened, now format the error message */
+static void
+NotNullViolationErrorReport(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));
+}
+
/*
* ExecConstraints - check constraints of the tuple in 'slot'
*
@@ -2058,73 +2196,63 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
TupleDesc tupdesc = RelationGetDescr(rel);
TupleConstr *constr = tupdesc->constr;
Bitmapset *modifiedCols;
+ bool virtual_notnull = false;
+ Form_pg_attribute att;
+ int natts;
+ int attnum;
Assert(constr); /* we should not be called otherwise */
+ natts = tupdesc->natts;
if (constr->has_not_null)
{
- int natts = tupdesc->natts;
- int attrChk;
-
- for (attrChk = 1; attrChk <= natts; attrChk++)
+ for (attnum = 1; attnum <= natts; attnum++)
{
- Form_pg_attribute att = TupleDescAttr(tupdesc, attrChk - 1);
+ att = TupleDescAttr(tupdesc, attnum - 1);
- if (att->attnotnull && slot_attisnull(slot, attrChk))
+ 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)
+ /* virtual generated column not null constraint handled below */
+ 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)
+ virtual_notnull = 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)));
+ NotNullViolationErrorReport(resultRelInfo, slot, estate, attnum);
}
}
}
+ /* check virtual generated column not null constraint */
+ if (virtual_notnull)
+ {
+ Bitmapset *vir_gencols = NULL;
+
+ Assert(constr->has_not_null);
+ Assert(constr->has_generated_virtual);
+
+ for (attnum = 1; attnum <= natts; attnum++)
+ {
+ att = TupleDescAttr(tupdesc, attnum - 1);
+ if (att->attnotnull && att->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+ vir_gencols = bms_add_member(vir_gencols, att->attnum);
+ }
+ attnum = -1;
+ Assert(vir_gencols != NULL);
+
+ attnum = ExecRelCheckGenVirtualNotNull(resultRelInfo, slot, estate, vir_gencols);
+
+ /* constraint evaulation return falsem do error report, also make an Assert */
+ if (attnum > 0)
+ {
+ att = TupleDescAttr(tupdesc, attnum - 1);
+ Assert(att->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL);
+
+ NotNullViolationErrorReport(resultRelInfo, slot, estate, attnum);
+ }
+ }
+
if (rel->rd_rel->relchecks > 0)
{
const char *failed;
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index abbe1bb45a3..7e9a5f7411b 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/nodes/execnodes.h b/src/include/nodes/execnodes.h
index a323fa98bbb..1c72243468c 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -549,6 +549,8 @@ 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 7ef05f45be7..3bd5130587c 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -664,28 +664,46 @@ 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
+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 (f2 * 2) 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 (1, NULL);
+ERROR: null value in column "f3" of relation "gtestnn_child" violates not-null constraint
+DETAIL: Failing row contains (1, null, virtual).
+INSERT INTO gtestnn_parent VALUES (5, NULL);
+ERROR: null value in column "f3" of relation "gtestnn_childdef" violates not-null constraint
+DETAIL: Failing row contains (5, null, virtual).
-- 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 +711,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 key on virtual generated columns are not supported
--INSERT INTO gtest22b VALUES (2);
--INSERT INTO gtest22b VALUES (2);
-- indexes
@@ -738,7 +756,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 key 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
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index dab8c92ef99..f436e8f2313 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -335,23 +335,36 @@ 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
+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 (f2 * 2) 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 (1, NULL);
+INSERT INTO gtestnn_parent VALUES (5, NULL);
+
-- index constraints
CREATE TABLE gtest22a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a / 2) VIRTUAL UNIQUE);
--
2.34.1
