From: Álvaro Herrera <alvherre@alvh.no-ip.org>
To: jian he <jian.universality@gmail.com>
Cc: Xuneng Zhou <xunengzhou@gmail.com>
Cc: Navneet Kumar <thanit3111@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: support virtual generated column not null constraint
Date: Wed, 19 Mar 2025 17:19:24 +0100
Message-ID: <202503191619.c72arfi34vid@alvherre.pgsql> (raw)
In-Reply-To: <CACJufxHT4R1oABzeQuvjb6DHrig7k-QSr6LEe53Q_nLi9pfanA@mail.gmail.com>
On 2025-Mar-13, jian he wrote:
> hi.
>
> new patch attached.
>
> 0001 for virtual generated columns not null.
> minor change to fix the compiler warning.
I gave a look at 0001, and I propose some fixes. 0001 is just a typo in
an error message. 0002 is pgindent. Then 0003 contain some more
interesting bits:
- in ExecRelCheckGenVirtualNotNull() it seems more natural to an
AttrNumber rather than int, and use the InvalidAttrNumber value rather
than -1 to indicate that all columns are ok. Also, use
foreach_current_index instead of manually counting the loop.
- ATRewriteTable can be simplified if we no longer add the virtual
generated columns with not nulls to the notnulls_attrs list, but instead
we store the attnums in a separate list. That way, the machinery
around ExecRelCheckGenVirtualNotNull made less convoluted.
- in ExecConstraints, you removed a comment (which ended up in
NotNullViolationErrorReport), which was OK as far as that went; but
there was another comment a few lines below which said "See the
comment above", which no longer made sense. To fix it, I duplicated
the original comment in the place where "See the..." comment was.
- renamed NotNullViolationErrorReport to ReportNotNullViolationError.
Perhaps a better name is still possible -- something in line with
ExecPartitionCheckEmitError? (However, that function is exported,
while the new one is not. So we probably don't need an "Exec" prefix
for it. I don't particularly like that name very much anyway.)
- Reduce scope of variables all over the place.
- Added a bunch more comments.
Other comments:
* The block in ATRewriteTable that creates a resultRelInfo for
ExecRelCheckGenVirtualNotNull needs an explanation.
* 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 don't find the name all_virtual_nns particularly appropriate. Maybe
virtual_notnull_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.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Having your biases confirmed independently is how scientific progress is
made, and hence made our great society what it is today" (Mary Gardiner)
From 01a9f9bebf0a1d19363ce8ac37ca4583fa809773 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@alvh.no-ip.org>
Date: Wed, 19 Mar 2025 12:04:25 +0100
Subject: [PATCH 1/3] fix typo in errmsg
---
src/backend/commands/indexcmds.c | 2 +-
src/test/regress/expected/generated_virtual.out | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.cindex f62ba12f868..33c2106c17c 100644--- a/src/backend/commands/indexcmds.c+++ b/src/backend/commands/indexcmds.c@@ -1120,7 +1120,7 @@ DefineIndex(Oid tableId,
ereport(ERROR,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
stmt->primary ?
- errmsg("primary key on virtual generated columns are not supported") :+ 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/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.outindex 3b5723599f1..98f2e486f88 100644--- a/src/test/regress/expected/generated_virtual.out+++ b/src/test/regress/expected/generated_virtual.out@@ -733,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: primary key on virtual generated columns are not supported+ERROR: primary keys on virtual generated columns are not supported
--INSERT INTO gtest22b VALUES (2);
--INSERT INTO gtest22b VALUES (2);
-- indexes
@@ -778,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: primary key on virtual generated columns are not supported+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
--
2.39.5
From ccd36ac6a786b1a29356e986e61a8c9a3fdf9cb7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@alvh.no-ip.org>
Date: Wed, 19 Mar 2025 12:04:54 +0100
Subject: [PATCH 2/3] pgindent
---
src/backend/commands/tablecmds.c | 18 +++++++++------
src/backend/executor/execMain.c | 39 +++++++++++++++++---------------
src/include/executor/executor.h | 8 +++----
src/include/nodes/execnodes.h | 1 +
4 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 07aa719065d..e49a4a4ad7d 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -6216,7 +6216,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
List *dropped_attrs = NIL;
ListCell *lc;
Snapshot snapshot;
- ResultRelInfo *rInfo = NULL;+ ResultRelInfo *rInfo = NULL;
MemoryContext oldcontext;
if (list_length(all_virtual_nns) > 0)
@@ -6227,7 +6227,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
rInfo = makeNode(ResultRelInfo);
InitResultRelInfo(rInfo,
oldrel,
- 0, /* dummy rangetable index */+ 0, /* dummy rangetable index */
NULL,
estate->es_instrument);
MemoryContextSwitchTo(oldcontext);
@@ -6314,7 +6314,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
while (table_scan_getnextslot(scan, ForwardScanDirection, oldslot))
{
TupleTableSlot *insertslot;
- bool virtual_notnull_check = false;+ bool virtual_notnull_check = false;
if (tab->rewrite > 0)
{
@@ -6406,7 +6406,10 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
{
Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn);
- /* virtual generated column not null constraint handled below */+ /*+ * virtual generated column not null constraint handled+ * below+ */
if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
{
@@ -6425,7 +6428,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
if (virtual_notnull_check)
{
- int attnum = -1;+ int attnum = -1;+
Assert(list_length(all_virtual_nns) > 0);
attnum = ExecRelCheckGenVirtualNotNull(rInfo, insertslot,
@@ -6438,8 +6442,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
ereport(ERROR,
errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("column \"%s\" of relation \"%s\" contains null values",
- NameStr(attr->attname),- RelationGetRelationName(oldrel)),+ NameStr(attr->attname),+ RelationGetRelationName(oldrel)),
errtablecol(oldrel, attnum));
}
}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.cindex 5852f272b44..32e1978a724 100644--- a/src/backend/executor/execMain.c+++ b/src/backend/executor/execMain.c@@ -1886,8 +1886,8 @@ ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo, TupleTableSlot *slot
/*
* 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).+ * generated column not null constraint expressions (creating it if it's+ * not already there).
*/
econtext = GetPerTupleExprContext(estate);
@@ -1899,6 +1899,7 @@ ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo, TupleTableSlot *slot
foreach_int(attnum, all_virtual_nns)
{
ExprState *exprstate = resultRelInfo->ri_GeneratedNotNullExprs[i++];
+
if (exprstate && !ExecCheck(exprstate, econtext))
return attnum;
}
@@ -2123,12 +2124,11 @@ NotNullViolationErrorReport(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
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 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;
@@ -2141,9 +2141,9 @@ NotNullViolationErrorReport(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
false);
/*
- * Partition-specific slot's tupdesc can't be changed, so- * allocate a new one.- */+ * 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));
@@ -2163,10 +2163,10 @@ NotNullViolationErrorReport(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
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));+ NameStr(att->attname),+ RelationGetRelationName(orig_rel)),+ val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,+ errtablecol(orig_rel, attrChk));
}
/*
@@ -2191,11 +2191,11 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
Form_pg_attribute att;
int natts;
int attnum;
- List *all_virtual_nns = NIL;+ List *all_virtual_nns = NIL;
bool virtual_notnull_check = false;
Assert(constr); /* we should not be called otherwise */
- natts = tupdesc->natts;+ natts = tupdesc->natts;
if (constr->has_not_null)
{
@@ -2229,7 +2229,10 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
attnum = -1;
attnum = ExecRelCheckGenVirtualNotNull(resultRelInfo, slot, estate, all_virtual_nns);
- /* constraint evaulation return falsem do error report, also make an Assert */+ /*+ * constraint evaulation return falsem do error report, also make an+ * Assert+ */
if (attnum > 0)
{
att = TupleDescAttr(tupdesc, attnum - 1);
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.hindex 549e8c9b095..7cfedd14c9f 100644--- a/src/include/executor/executor.h+++ b/src/include/executor/executor.h@@ -220,10 +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 int ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo,- TupleTableSlot *slot,- EState *estate,- List *all_virtual_nns);+extern int ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo,+ TupleTableSlot *slot,+ EState *estate,+ List *all_virtual_nns);
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.hindex af8f8a1e999..6159b8a3c07 100644--- a/src/include/nodes/execnodes.h+++ b/src/include/nodes/execnodes.h@@ -551,6 +551,7 @@ typedef struct ResultRelInfo
/* array of virtual generated not null constraint-checking expr states */
ExprState **ri_GeneratedNotNullExprs;
+
/*
* Arrays of stored generated columns ExprStates for INSERT/UPDATE/MERGE.
*/
--
2.39.5
From fd18867d91b2fedce631b0f6574d13216016264e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@alvh.no-ip.org>
Date: Wed, 19 Mar 2025 16:46:58 +0100
Subject: [PATCH 3/3] Some code review
---
src/backend/commands/tablecmds.c | 57 +++++++----------
src/backend/executor/execMain.c | 105 ++++++++++++++++++-------------
src/include/executor/executor.h | 8 +--
3 files changed, 89 insertions(+), 81 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex e49a4a4ad7d..32890fa5453 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -6092,7 +6092,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
TupleDesc newTupDesc;
bool needscan = false;
List *notnull_attrs;
- List *all_virtual_nns = NIL;+ List *notnull_virtual_attrs;
int i;
ListCell *l;
EState *estate;
@@ -6177,35 +6177,33 @@ 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;
}
- foreach_int(attn, notnull_attrs)- {- Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn);-- Assert(attr->attnotnull);- Assert(!attr->attisdropped);- if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)- all_virtual_nns = lappend_int(all_virtual_nns, attr->attnum);- }-
if (newrel || needscan)
{
ExprContext *econtext;
@@ -6217,10 +6215,15 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
ListCell *lc;
Snapshot snapshot;
ResultRelInfo *rInfo = NULL;
- MemoryContext oldcontext;- if (list_length(all_virtual_nns) > 0)+ /*+ * XXX this deserves an explanation. Also, is rInfo a good variable+ * name?+ */+ 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);
@@ -6314,7 +6317,6 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
while (table_scan_getnextslot(scan, ForwardScanDirection, oldslot))
{
TupleTableSlot *insertslot;
- bool virtual_notnull_check = false;
if (tab->rewrite > 0)
{
@@ -6406,17 +6408,6 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
{
Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn);
- /*- * virtual generated column not null constraint handled- * below- */-- if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)- {- if (!virtual_notnull_check)- virtual_notnull_check = true;- continue;- }
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("column \"%s\" of relation \"%s\" contains null values",
@@ -6426,16 +6417,14 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
}
}
- if (virtual_notnull_check)+ if (notnull_virtual_attrs != NIL)
{
- int attnum = -1;-- Assert(list_length(all_virtual_nns) > 0);+ AttrNumber attnum;
attnum = ExecRelCheckGenVirtualNotNull(rInfo, insertslot,
estate,
- all_virtual_nns);- if (attnum > 0)+ notnull_virtual_attrs);+ if (attnum != InvalidAttrNumber)
{
Form_pg_attribute attr = TupleDescAttr(newTupDesc, attnum - 1);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.cindex 32e1978a724..0cefb88aaba 100644--- a/src/backend/executor/execMain.c+++ b/src/backend/executor/execMain.c@@ -1842,44 +1842,54 @@ ExecutePlan(QueryDesc *queryDesc,
ExitParallelMode();
}
-
/*
- * we warp "virtual_generated col IS NOT NULL" to a NullTest node.- * NullTest->arg is the virtual generated expression. return value of -1 means- * ok. return value > 0 means not null violation happended for that attribute- * number.- * all_virtual_nns is *all* the virtual generated column not-null attribute numbers.+ * Verify not-null constraints on virtual generated columns of the given+ * tuple slot.
*
-*/-int+ * 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.+ *+ * all_virtual_nns is the list of the attnums of virtual generated column with+ * not-null constraints.+ */+AttrNumber
ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
EState *estate, List *all_virtual_nns)
{
Relation rel = resultRelInfo->ri_RelationDesc;
ExprContext *econtext;
MemoryContext oldContext;
- int i = 0;- int cnt = list_length(all_virtual_nns);++ /*+ * We implement this by consing up a NullTest node for each virtual+ * generated column, which we cache in resultRelInfo, and running those+ * through ExecCheck.+ *+ * XXX are there cases where we need ->argisrow = true?+ */
if (resultRelInfo->ri_GeneratedNotNullExprs == NULL)
{
+ int cnt = list_length(all_virtual_nns);+
oldContext = MemoryContextSwitchTo(estate->es_query_cxt);
resultRelInfo->ri_GeneratedNotNullExprs =
(ExprState **) palloc0(cnt * sizeof(ExprState *));
- foreach_int(attidx, all_virtual_nns)+ foreach_int(attnum, all_virtual_nns)
{
- Expr *expr;+ int i = foreach_current_index(attnum);
NullTest *nnulltest;
nnulltest = makeNode(NullTest);
- nnulltest->arg = (Expr *) build_generation_expression(rel, attidx);+ nnulltest->arg = (Expr *) build_generation_expression(rel, attnum);
nnulltest->nulltesttype = IS_NOT_NULL;
nnulltest->argisrow = false;
nnulltest->location = -1;
- expr = (Expr *) nnulltest;- resultRelInfo->ri_GeneratedNotNullExprs[i++] = ExecPrepareExpr(expr, estate);+ resultRelInfo->ri_GeneratedNotNullExprs[i] =+ ExecPrepareExpr((Expr *) nnulltest, estate);
}
MemoryContextSwitchTo(oldContext);
}
@@ -1895,17 +1905,18 @@ ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo, TupleTableSlot *slot
econtext->ecxt_scantuple = slot;
/* And evaluate the check constraints for virtual generated column */
- i = 0;
foreach_int(attnum, all_virtual_nns)
{
- ExprState *exprstate = resultRelInfo->ri_GeneratedNotNullExprs[i++];+ int i = foreach_current_index(attnum);+ ExprState *exprstate = resultRelInfo->ri_GeneratedNotNullExprs[i];- if (exprstate && !ExecCheck(exprstate, econtext))+ Assert(exprstate != NULL);+ if (!ExecCheck(exprstate, econtext))
return attnum;
}
- /* -1 result means no error */- return -1;+ /* InvalidAttrNumber result means no error */+ return InvalidAttrNumber;
}
/*
@@ -2106,9 +2117,11 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
errtable(resultRelInfo->ri_RelationDesc)));
}
-/* not null constraint violation happened, now format the error message */+/*+ * Report a violation of a not-null constraint that was already detected.+ */
static void
-NotNullViolationErrorReport(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,+ReportNotNullViolationError(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
EState *estate, int attrChk)
{
Bitmapset *modifiedCols;
@@ -2188,20 +2201,22 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
TupleDesc tupdesc = RelationGetDescr(rel);
TupleConstr *constr = tupdesc->constr;
Bitmapset *modifiedCols;
- Form_pg_attribute att;- int natts;- int attnum;
List *all_virtual_nns = NIL;
bool virtual_notnull_check = false;
Assert(constr); /* we should not be called otherwise */
- natts = tupdesc->natts;+ /*+ * 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)
{
- for (attnum = 1; attnum <= natts; attnum++)+ for (AttrNumber attnum = 1; attnum <= tupdesc->natts; attnum++)
{
- att = TupleDescAttr(tupdesc, attnum - 1);+ Form_pg_attribute att = TupleDescAttr(tupdesc, attnum - 1);
if (att->attnotnull && att->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
all_virtual_nns = lappend_int(all_virtual_nns, att->attnum);
@@ -2214,34 +2229,33 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
virtual_notnull_check = true;
continue;
}
- NotNullViolationErrorReport(resultRelInfo, slot, estate, attnum);++ ReportNotNullViolationError(resultRelInfo, slot, estate, attnum);
}
}
}
- /* check virtual generated column not null constraint */+ /* 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(list_length(all_virtual_nns) > 0);+ Assert(all_virtual_nns != NIL);- attnum = -1;- attnum = ExecRelCheckGenVirtualNotNull(resultRelInfo, slot, estate, all_virtual_nns);-- /*- * constraint evaulation return falsem do error report, also make an- * Assert- */- if (attnum > 0)+ attnum = ExecRelCheckGenVirtualNotNull(resultRelInfo, slot, estate,+ all_virtual_nns);+ if (attnum != InvalidAttrNumber)
{
- att = TupleDescAttr(tupdesc, attnum - 1);- Assert(att->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL);+ Form_pg_attribute att = TupleDescAttr(tupdesc, attnum - 1);- NotNullViolationErrorReport(resultRelInfo, slot, estate, att->attnum);+ 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;
@@ -2251,7 +2265,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;
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.hindex 7cfedd14c9f..cd7f2266af7 100644--- a/src/include/executor/executor.h+++ b/src/include/executor/executor.h@@ -220,10 +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 int ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo,- TupleTableSlot *slot,- EState *estate,- List *all_virtual_nns);+extern AttrNumber ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo,+ TupleTableSlot *slot,+ EState *estate,+ List *all_virtual_nns);
extern bool ExecPartitionCheck(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate, bool emitError);
extern void ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
--
2.39.5
Attachments:
[text/plain] 0001-fix-typo-in-errmsg.patch.txt (2.3K, ../202503191619.c72arfi34vid@alvherre.pgsql/2-0001-fix-typo-in-errmsg.patch.txt)
download | inline diff:
From 01a9f9bebf0a1d19363ce8ac37ca4583fa809773 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@alvh.no-ip.org>
Date: Wed, 19 Mar 2025 12:04:25 +0100
Subject: [PATCH 1/3] fix typo in errmsg
---
src/backend/commands/indexcmds.c | 2 +-
src/test/regress/expected/generated_virtual.out | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.cindex f62ba12f868..33c2106c17c 100644--- a/src/backend/commands/indexcmds.c+++ b/src/backend/commands/indexcmds.c@@ -1120,7 +1120,7 @@ DefineIndex(Oid tableId,
ereport(ERROR,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
stmt->primary ?
- errmsg("primary key on virtual generated columns are not supported") :+ 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/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.outindex 3b5723599f1..98f2e486f88 100644--- a/src/test/regress/expected/generated_virtual.out+++ b/src/test/regress/expected/generated_virtual.out@@ -733,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: primary key on virtual generated columns are not supported+ERROR: primary keys on virtual generated columns are not supported
--INSERT INTO gtest22b VALUES (2);
--INSERT INTO gtest22b VALUES (2);
-- indexes
@@ -778,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: primary key on virtual generated columns are not supported+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
--
2.39.5
[text/plain] 0002-pgindent.patch.txt (7.5K, ../202503191619.c72arfi34vid@alvherre.pgsql/3-0002-pgindent.patch.txt)
download | inline diff:
From ccd36ac6a786b1a29356e986e61a8c9a3fdf9cb7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@alvh.no-ip.org>
Date: Wed, 19 Mar 2025 12:04:54 +0100
Subject: [PATCH 2/3] pgindent
---
src/backend/commands/tablecmds.c | 18 +++++++++------
src/backend/executor/execMain.c | 39 +++++++++++++++++---------------
src/include/executor/executor.h | 8 +++----
src/include/nodes/execnodes.h | 1 +
4 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 07aa719065d..e49a4a4ad7d 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -6216,7 +6216,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
List *dropped_attrs = NIL;
ListCell *lc;
Snapshot snapshot;
- ResultRelInfo *rInfo = NULL;+ ResultRelInfo *rInfo = NULL;
MemoryContext oldcontext;
if (list_length(all_virtual_nns) > 0)
@@ -6227,7 +6227,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
rInfo = makeNode(ResultRelInfo);
InitResultRelInfo(rInfo,
oldrel,
- 0, /* dummy rangetable index */+ 0, /* dummy rangetable index */
NULL,
estate->es_instrument);
MemoryContextSwitchTo(oldcontext);
@@ -6314,7 +6314,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
while (table_scan_getnextslot(scan, ForwardScanDirection, oldslot))
{
TupleTableSlot *insertslot;
- bool virtual_notnull_check = false;+ bool virtual_notnull_check = false;
if (tab->rewrite > 0)
{
@@ -6406,7 +6406,10 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
{
Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn);
- /* virtual generated column not null constraint handled below */+ /*+ * virtual generated column not null constraint handled+ * below+ */
if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
{
@@ -6425,7 +6428,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
if (virtual_notnull_check)
{
- int attnum = -1;+ int attnum = -1;+
Assert(list_length(all_virtual_nns) > 0);
attnum = ExecRelCheckGenVirtualNotNull(rInfo, insertslot,
@@ -6438,8 +6442,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
ereport(ERROR,
errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("column \"%s\" of relation \"%s\" contains null values",
- NameStr(attr->attname),- RelationGetRelationName(oldrel)),+ NameStr(attr->attname),+ RelationGetRelationName(oldrel)),
errtablecol(oldrel, attnum));
}
}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.cindex 5852f272b44..32e1978a724 100644--- a/src/backend/executor/execMain.c+++ b/src/backend/executor/execMain.c@@ -1886,8 +1886,8 @@ ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo, TupleTableSlot *slot
/*
* 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).+ * generated column not null constraint expressions (creating it if it's+ * not already there).
*/
econtext = GetPerTupleExprContext(estate);
@@ -1899,6 +1899,7 @@ ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo, TupleTableSlot *slot
foreach_int(attnum, all_virtual_nns)
{
ExprState *exprstate = resultRelInfo->ri_GeneratedNotNullExprs[i++];
+
if (exprstate && !ExecCheck(exprstate, econtext))
return attnum;
}
@@ -2123,12 +2124,11 @@ NotNullViolationErrorReport(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
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 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;
@@ -2141,9 +2141,9 @@ NotNullViolationErrorReport(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
false);
/*
- * Partition-specific slot's tupdesc can't be changed, so- * allocate a new one.- */+ * 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));
@@ -2163,10 +2163,10 @@ NotNullViolationErrorReport(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
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));+ NameStr(att->attname),+ RelationGetRelationName(orig_rel)),+ val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,+ errtablecol(orig_rel, attrChk));
}
/*
@@ -2191,11 +2191,11 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
Form_pg_attribute att;
int natts;
int attnum;
- List *all_virtual_nns = NIL;+ List *all_virtual_nns = NIL;
bool virtual_notnull_check = false;
Assert(constr); /* we should not be called otherwise */
- natts = tupdesc->natts;+ natts = tupdesc->natts;
if (constr->has_not_null)
{
@@ -2229,7 +2229,10 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
attnum = -1;
attnum = ExecRelCheckGenVirtualNotNull(resultRelInfo, slot, estate, all_virtual_nns);
- /* constraint evaulation return falsem do error report, also make an Assert */+ /*+ * constraint evaulation return falsem do error report, also make an+ * Assert+ */
if (attnum > 0)
{
att = TupleDescAttr(tupdesc, attnum - 1);
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.hindex 549e8c9b095..7cfedd14c9f 100644--- a/src/include/executor/executor.h+++ b/src/include/executor/executor.h@@ -220,10 +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 int ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo,- TupleTableSlot *slot,- EState *estate,- List *all_virtual_nns);+extern int ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo,+ TupleTableSlot *slot,+ EState *estate,+ List *all_virtual_nns);
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.hindex af8f8a1e999..6159b8a3c07 100644--- a/src/include/nodes/execnodes.h+++ b/src/include/nodes/execnodes.h@@ -551,6 +551,7 @@ typedef struct ResultRelInfo
/* array of virtual generated not null constraint-checking expr states */
ExprState **ri_GeneratedNotNullExprs;
+
/*
* Arrays of stored generated columns ExprStates for INSERT/UPDATE/MERGE.
*/
--
2.39.5
[text/plain] 0003-Some-code-review.patch.txt (12.1K, ../202503191619.c72arfi34vid@alvherre.pgsql/4-0003-Some-code-review.patch.txt)
download | inline diff:
From fd18867d91b2fedce631b0f6574d13216016264e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@alvh.no-ip.org>
Date: Wed, 19 Mar 2025 16:46:58 +0100
Subject: [PATCH 3/3] Some code review
---
src/backend/commands/tablecmds.c | 57 +++++++----------
src/backend/executor/execMain.c | 105 ++++++++++++++++++-------------
src/include/executor/executor.h | 8 +--
3 files changed, 89 insertions(+), 81 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex e49a4a4ad7d..32890fa5453 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -6092,7 +6092,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
TupleDesc newTupDesc;
bool needscan = false;
List *notnull_attrs;
- List *all_virtual_nns = NIL;+ List *notnull_virtual_attrs;
int i;
ListCell *l;
EState *estate;
@@ -6177,35 +6177,33 @@ 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;
}
- foreach_int(attn, notnull_attrs)- {- Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn);-- Assert(attr->attnotnull);- Assert(!attr->attisdropped);- if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)- all_virtual_nns = lappend_int(all_virtual_nns, attr->attnum);- }-
if (newrel || needscan)
{
ExprContext *econtext;
@@ -6217,10 +6215,15 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
ListCell *lc;
Snapshot snapshot;
ResultRelInfo *rInfo = NULL;
- MemoryContext oldcontext;- if (list_length(all_virtual_nns) > 0)+ /*+ * XXX this deserves an explanation. Also, is rInfo a good variable+ * name?+ */+ 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);
@@ -6314,7 +6317,6 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
while (table_scan_getnextslot(scan, ForwardScanDirection, oldslot))
{
TupleTableSlot *insertslot;
- bool virtual_notnull_check = false;
if (tab->rewrite > 0)
{
@@ -6406,17 +6408,6 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
{
Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn);
- /*- * virtual generated column not null constraint handled- * below- */-- if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)- {- if (!virtual_notnull_check)- virtual_notnull_check = true;- continue;- }
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("column \"%s\" of relation \"%s\" contains null values",
@@ -6426,16 +6417,14 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
}
}
- if (virtual_notnull_check)+ if (notnull_virtual_attrs != NIL)
{
- int attnum = -1;-- Assert(list_length(all_virtual_nns) > 0);+ AttrNumber attnum;
attnum = ExecRelCheckGenVirtualNotNull(rInfo, insertslot,
estate,
- all_virtual_nns);- if (attnum > 0)+ notnull_virtual_attrs);+ if (attnum != InvalidAttrNumber)
{
Form_pg_attribute attr = TupleDescAttr(newTupDesc, attnum - 1);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.cindex 32e1978a724..0cefb88aaba 100644--- a/src/backend/executor/execMain.c+++ b/src/backend/executor/execMain.c@@ -1842,44 +1842,54 @@ ExecutePlan(QueryDesc *queryDesc,
ExitParallelMode();
}
-
/*
- * we warp "virtual_generated col IS NOT NULL" to a NullTest node.- * NullTest->arg is the virtual generated expression. return value of -1 means- * ok. return value > 0 means not null violation happended for that attribute- * number.- * all_virtual_nns is *all* the virtual generated column not-null attribute numbers.+ * Verify not-null constraints on virtual generated columns of the given+ * tuple slot.
*
-*/-int+ * 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.+ *+ * all_virtual_nns is the list of the attnums of virtual generated column with+ * not-null constraints.+ */+AttrNumber
ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
EState *estate, List *all_virtual_nns)
{
Relation rel = resultRelInfo->ri_RelationDesc;
ExprContext *econtext;
MemoryContext oldContext;
- int i = 0;- int cnt = list_length(all_virtual_nns);++ /*+ * We implement this by consing up a NullTest node for each virtual+ * generated column, which we cache in resultRelInfo, and running those+ * through ExecCheck.+ *+ * XXX are there cases where we need ->argisrow = true?+ */
if (resultRelInfo->ri_GeneratedNotNullExprs == NULL)
{
+ int cnt = list_length(all_virtual_nns);+
oldContext = MemoryContextSwitchTo(estate->es_query_cxt);
resultRelInfo->ri_GeneratedNotNullExprs =
(ExprState **) palloc0(cnt * sizeof(ExprState *));
- foreach_int(attidx, all_virtual_nns)+ foreach_int(attnum, all_virtual_nns)
{
- Expr *expr;+ int i = foreach_current_index(attnum);
NullTest *nnulltest;
nnulltest = makeNode(NullTest);
- nnulltest->arg = (Expr *) build_generation_expression(rel, attidx);+ nnulltest->arg = (Expr *) build_generation_expression(rel, attnum);
nnulltest->nulltesttype = IS_NOT_NULL;
nnulltest->argisrow = false;
nnulltest->location = -1;
- expr = (Expr *) nnulltest;- resultRelInfo->ri_GeneratedNotNullExprs[i++] = ExecPrepareExpr(expr, estate);+ resultRelInfo->ri_GeneratedNotNullExprs[i] =+ ExecPrepareExpr((Expr *) nnulltest, estate);
}
MemoryContextSwitchTo(oldContext);
}
@@ -1895,17 +1905,18 @@ ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo, TupleTableSlot *slot
econtext->ecxt_scantuple = slot;
/* And evaluate the check constraints for virtual generated column */
- i = 0;
foreach_int(attnum, all_virtual_nns)
{
- ExprState *exprstate = resultRelInfo->ri_GeneratedNotNullExprs[i++];+ int i = foreach_current_index(attnum);+ ExprState *exprstate = resultRelInfo->ri_GeneratedNotNullExprs[i];- if (exprstate && !ExecCheck(exprstate, econtext))+ Assert(exprstate != NULL);+ if (!ExecCheck(exprstate, econtext))
return attnum;
}
- /* -1 result means no error */- return -1;+ /* InvalidAttrNumber result means no error */+ return InvalidAttrNumber;
}
/*
@@ -2106,9 +2117,11 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
errtable(resultRelInfo->ri_RelationDesc)));
}
-/* not null constraint violation happened, now format the error message */+/*+ * Report a violation of a not-null constraint that was already detected.+ */
static void
-NotNullViolationErrorReport(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,+ReportNotNullViolationError(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
EState *estate, int attrChk)
{
Bitmapset *modifiedCols;
@@ -2188,20 +2201,22 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
TupleDesc tupdesc = RelationGetDescr(rel);
TupleConstr *constr = tupdesc->constr;
Bitmapset *modifiedCols;
- Form_pg_attribute att;- int natts;- int attnum;
List *all_virtual_nns = NIL;
bool virtual_notnull_check = false;
Assert(constr); /* we should not be called otherwise */
- natts = tupdesc->natts;+ /*+ * 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)
{
- for (attnum = 1; attnum <= natts; attnum++)+ for (AttrNumber attnum = 1; attnum <= tupdesc->natts; attnum++)
{
- att = TupleDescAttr(tupdesc, attnum - 1);+ Form_pg_attribute att = TupleDescAttr(tupdesc, attnum - 1);
if (att->attnotnull && att->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
all_virtual_nns = lappend_int(all_virtual_nns, att->attnum);
@@ -2214,34 +2229,33 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
virtual_notnull_check = true;
continue;
}
- NotNullViolationErrorReport(resultRelInfo, slot, estate, attnum);++ ReportNotNullViolationError(resultRelInfo, slot, estate, attnum);
}
}
}
- /* check virtual generated column not null constraint */+ /* 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(list_length(all_virtual_nns) > 0);+ Assert(all_virtual_nns != NIL);- attnum = -1;- attnum = ExecRelCheckGenVirtualNotNull(resultRelInfo, slot, estate, all_virtual_nns);-- /*- * constraint evaulation return falsem do error report, also make an- * Assert- */- if (attnum > 0)+ attnum = ExecRelCheckGenVirtualNotNull(resultRelInfo, slot, estate,+ all_virtual_nns);+ if (attnum != InvalidAttrNumber)
{
- att = TupleDescAttr(tupdesc, attnum - 1);- Assert(att->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL);+ Form_pg_attribute att = TupleDescAttr(tupdesc, attnum - 1);- NotNullViolationErrorReport(resultRelInfo, slot, estate, att->attnum);+ 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;
@@ -2251,7 +2265,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;
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.hindex 7cfedd14c9f..cd7f2266af7 100644--- a/src/include/executor/executor.h+++ b/src/include/executor/executor.h@@ -220,10 +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 int ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo,- TupleTableSlot *slot,- EState *estate,- List *all_virtual_nns);+extern AttrNumber ExecRelCheckGenVirtualNotNull(ResultRelInfo *resultRelInfo,+ TupleTableSlot *slot,+ EState *estate,+ List *all_virtual_nns);
extern bool ExecPartitionCheck(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate, bool emitError);
extern void ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
--
2.39.5
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, jian.universality@gmail.com, xunengzhou@gmail.com, thanit3111@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: support virtual generated column not null constraint
In-Reply-To: <202503191619.c72arfi34vid@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox