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?= <alvhe...@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.c
index 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.out
index 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?= <alvhe...@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.c
index 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.c
index 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.h
index 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.h
index 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?= <alvhe...@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.c
index 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.c
index 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.h
index 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
