Thread: Re: support virtual generated column not null constraint
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.
/* And evaluate the check constraints for virtual generated column */
+ for (i = 0; i < bms_num_members(gen_virtual_cols); i++)
+ {
+ ExprState *gen_virtualnn = resultRelInfo->ri_VirGeneratedConstraintExprs[i];
+
+ if (gen_virtualnn && !ExecCheck(gen_virtualnn, econtext))
+ return attnums[i];
+ }
+
/* And evaluate the constraints */
for (i = 0; i < ncheck; i++)
{
ExprState *checkconstr = resultRelInfo->ri_ConstraintExprs[i];
/*
* NOTE: SQL specifies that a NULL result from a constraint expression
* is not to be treated as a failure. Therefore, use ExecCheck not
* ExecQual.
*/
if (checkconstr && !ExecCheck(checkconstr, econtext))
return check[i].ccname;
}
/* NULL result means no error */
return NULL;
Best regards,
Xuneng
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
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
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
2. The naming of variable gen_virtualnn.
Gen_virtualnn looks confusing at first glance. Checkconstr seems to be more straightforward.
+ 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.2. The naming of variable gen_virtualnn.
Gen_virtualnn looks confusing at first glance. Checkconstr seems to be more straightforward.
/* And evaluate the check constraints for virtual generated column */
+ for (i = 0; i < bms_num_members(gen_virtual_cols); i++)
+ {
+ ExprState *gen_virtualnn = resultRelInfo->ri_VirGeneratedConstraintExprs[i];
+
+ if (gen_virtualnn && !ExecCheck(gen_virtualnn, econtext))
+ return attnums[i];
+ }
+
/* And evaluate the constraints */
for (i = 0; i < ncheck; i++)
{
ExprState *checkconstr = resultRelInfo->ri_ConstraintExprs[i];
/*
* NOTE: SQL specifies that a NULL result from a constraint expression
* is not to be treated as a failure. Therefore, use ExecCheck not
* ExecQual.
*/
if (checkconstr && !ExecCheck(checkconstr, econtext))
return check[i].ccname;
}
/* NULL result means no error */
return NULL;
Best regards,
Xuneng
jian he <jian.universality@gmail.com> 于2025年2月10日周一 21:34写道：
hi.
Virtual generated columns committed,
https://git.postgresql.org/cgit/postgresql.git/commit/?id=83ea6c54025bea67bcd4949a6d58d3fc11c3e21b
This patch is for implementing not null constraints on virtual
generated columns.
NOT NULL constraints on virtual generated columns mean that
if we INSERT a row into the table and the evaluation of the generated
expression results in a null value,
an ERRCODE_NOT_NULL_VIOLATION error will be reported.
main gotcha is in ExecConstraints, expand the generated expression
and convert a not null constraint to a check constraint and evaluate it.
On Wed, Feb 26, 2025 at 3:01 PM ego alter <xunengzhou@gmail.com> wrote:
>
> Hi, I’ve had a chance to review the patch. As I am still getting familiar with executor part, questions and
feedbackscould 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
suggestsit’s used specifically for not null constraint-checking, but could it be extended to handle other constraints
inthe future as well? I assume one benefit is that it separates the handling of normal constraints from virtual ones,
butI'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.
Attachment
Hi
The above ALTER command works if I use STORED instead. Also if I define virtual generated column while creating table it works with same data.
1. CREATE TABLE person (
id INT GENERATED BY DEFAULT AS IDENTITY,
first_name VARCHAR(50) NOT NULL,
last_name VARCHAR(50) NOT NULL,
full_name VARCHAR(100) NOT NULL GENERATED ALWAYS AS (first_name || ' ' || last_name) VIRTUAL
);
2. INSERT INTO person(first_name, last_name)
VALUES ('first', 'last');
I encountered an issue when trying to add a virtual column to an existing table using the
ALTER command. The operation fails even though the existing data ensures that the virtual column's value will never be NULL. However, if I define the same virtual column while creating the table and then insert the same data, it works as expected.For example
This scenario fails
This scenario fails
1. CREATE TABLE person (
id INT GENERATED BY DEFAULT AS IDENTITY,
first_name VARCHAR(50) NOT NULL,
last_name VARCHAR(50) NOT NULL
);
2. INSERT INTO person (first_name, last_name)
VALUES ('first', 'last');
3. ALTER TABLE person
ADD COLUMN full_name VARCHAR(100) GENERATED ALWAYS AS (first_name || ' ' || last_name) VIRTUAL;
id INT GENERATED BY DEFAULT AS IDENTITY,
first_name VARCHAR(50) NOT NULL,
last_name VARCHAR(50) NOT NULL
);
2. INSERT INTO person (first_name, last_name)
VALUES ('first', 'last');
3. ALTER TABLE person
ADD COLUMN full_name VARCHAR(100) GENERATED ALWAYS AS (first_name || ' ' || last_name) VIRTUAL;
The above ALTER command works if I use STORED instead. Also if I define virtual generated column while creating table it works with same data.
1. CREATE TABLE person (
id INT GENERATED BY DEFAULT AS IDENTITY,
first_name VARCHAR(50) NOT NULL,
last_name VARCHAR(50) NOT NULL,
full_name VARCHAR(100) NOT NULL GENERATED ALWAYS AS (first_name || ' ' || last_name) VIRTUAL
);
2. INSERT INTO person(first_name, last_name)
VALUES ('first', 'last');
On Thu, Mar 6, 2025 at 7:15 PM jian he <jian.universality@gmail.com> wrote:
On Wed, Feb 26, 2025 at 3:01 PM ego alter <xunengzhou@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.
This scenario fails1. CREATE TABLE person (
id INT GENERATED BY DEFAULT AS IDENTITY,
first_name VARCHAR(50) NOT NULL,
last_name VARCHAR(50) NOT NULL
);
2. INSERT INTO person (first_name, last_name)
VALUES ('first', 'last');
3. ALTER TABLE person
ADD COLUMN full_name VARCHAR(100) GENERATED ALWAYS AS (first_name || ' ' || last_name) VIRTUAL;
Forgot to mention NOT NULL constraint in above query.
3. ALTER TABLE person
ADD COLUMN full_name VARCHAR(100) NOT NULL GENERATED ALWAYS AS (first_name || ' ' || last_name) VIRTUAL;
ERROR: column "full_name" of relation "person" contains null values
Hi,
jian he <jian.universality@gmail.com> 于2025年3月6日周四 21:44写道：
On Wed, Feb 26, 2025 at 3:01 PM ego alter <xunengzhou@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.
Thank you for your explanation! This name seems to be more clear for its usage.
> 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.
Hi,
forget to add hackers to cc.
Xuneng Zhou <xunengzhou@gmail.com> 于2025年3月8日周六 12:10写道：
Navneet Kumar <thanit3111@gmail.com> 于2025年3月8日周六 02:09写道：This scenario fails1. CREATE TABLE person (
id INT GENERATED BY DEFAULT AS IDENTITY,
first_name VARCHAR(50) NOT NULL,
last_name VARCHAR(50) NOT NULL
);
2. INSERT INTO person (first_name, last_name)
VALUES ('first', 'last');
3. ALTER TABLE person
ADD COLUMN full_name VARCHAR(100) GENERATED ALWAYS AS (first_name || ' ' || last_name) VIRTUAL;
Forgot to mention NOT NULL constraint in above query.
3. ALTER TABLE person
ADD COLUMN full_name VARCHAR(100) NOT NULL GENERATED ALWAYS AS (first_name || ' ' || last_name) VIRTUAL;
ERROR: column "full_name" of relation "person" contains null values
I did some debugging for this error. It is reported in this function:/*
* ATRewriteTable: scan or rewrite one table
*
* A rewrite is requested by passing a valid OIDNewHeap; in that case, caller
* must already hold AccessExclusiveLock on it.
*/
static void
ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
{
.......
/* Now check any constraints on the possibly-changed tuple */
econtext->ecxt_scantuple = insertslot;
foreach(l, notnull_attrs)
{
int attn = lfirst_int(l);
if (slot_attisnull(insertslot, attn + 1))
{
Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn);
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("column \"%s\" of relation \"%s\" contains null values",
NameStr(attr->attname),
RelationGetRelationName(oldrel)),
errtablecol(oldrel, attn + 1)));
}
}
.......
}
If this error is unexpected, I think the issue is that when adding a NOT NULL constraint to a regular column, pg scans the table to ensure no NULL values exist. But for virtual columns, there are no stored values to scan. Maybe we should add some condition like this? Then checking not null at runtime.
/* Skip NOT NULL validation for virtual generated columns during table rewrite */
if (TupleDescAttr(newTupDesc, attn)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
continue;
hi. patch attached to fix the above thread mentioned issue, related tests added. also there are several compiler warnings at [0] for virtual generated column domain patch, i did some minor adjustment, let's see how it goes. [0] https://cirrus-ci.com/task/4550600270020608
Attachment
hi.
new patch attached.
0001 for virtual generated columns not null.
minor change to fix the compiler warning.
0002-0004 is for domain over virtual generated columns.
0002: we need to compute the generation expression for the domain with
constraints,
thus rename ExecComputeStoredGenerated to ExecComputeGenerated.
0003. preparatory patch for 0004.
soft error variant of ExecPrepareExpr, ExecInitExpr.
for soft error processing of CoerceToDomain. see below description.
0004. no table rewrite for adding virtual generation column over
domain with constraints.
(syntax: ALTER TABLE xx ADD COLUMN X domain_type GENERATED ALWAYS AS
(expression) VIRTUAL
in phase3, ATRewriteTable: we already initialized AlteredTableInfo->newvals via
``ex->exprstate = ExecInitExpr((Expr *) ex->expr, NULL);``
we can easily evaluate it via ExecCheck. if fail, then error out.
--------------
reason for the 0003 patch:
ALTER DOMAIN ADD CONSTRAINT.
since the virtual generated column has no actual storage.
so, in validateDomainCheckConstraint we cannot use
```
d = slot_getattr(slot, attnum, &isNull);
econtext->domainValue_datum = d;
econtext->domainValue_isNull = isNull;
conResult = ExecEvalExprSwitchContext(exprstate,
econtext,
&isNull);
```
to check whether existing generation expression satisfy the newly
added domain constraint or not.
we also need to evaluate error softly,
because
ALTER DOMAIN ADD CONSTRAINT need check exists table domain value satisfy
the newly constraint or not.
if we not do soft error evaluation, the error message would be like:
``value for domain gtestdomain1 violates check constraint "gtestdomain1_check"``
but we want error message like:
ERROR: column "b" of table "gtest24" contains values that violate the
new constraint
--------------
Attachment
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)
Attachment
On Thu, Mar 20, 2025 at 12:19 AM Álvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> Other comments:
>
> * The block in ATRewriteTable that creates a resultRelInfo for
> ExecRelCheckGenVirtualNotNull needs an explanation.
>
I tried my best.
here are the comments above the line ``if (notnull_virtual_attrs != NIL)``
/*
* Whether change an existing generation expression or adding a new
* not-null virtual generated column, we need to evaluate whether the
* generation expression is null.
* In ExecConstraints, we use ExecRelCheckGenVirtualNotNull do the job.
* Here, we can also utilize ExecRelCheckGenVirtualNotNull.
* To achieve this, we need create a dummy ResultRelInfo. Ensure that
* the resultRelationIndex of the dummy ResultRelInfo is set to 0.
*/
+ /*
+ * XXX this deserves an explanation. Also, is rInfo a good variable
+ * name?
+ */
there are other places using this variable name: "rInfo", so i use
these names....
ATRewriteTable:
for (i = 0; i < newTupDesc->natts; i++)
{
Form_pg_attribute attr = TupleDescAttr(newTupDesc, i);
if (attr->attnotnull && !attr->attisdropped)
{
if (attr->attgenerated != ATTRIBUTE_GENERATED_VIRTUAL)
notnull_attrs = lappend_int(notnull_attrs, i);
else
notnull_virtual_attrs = lappend_int(notnull_virtual_attrs,
attr->attnum);
}
}
this is kind of ugly? notnull_virtual_attrs is 1 based, notnull_attrs
is 0 based.
I want to change it all to 1 based. see v5-0002
> * I suspect the locations for the new functions were selected with
> the help of a dartboard. ExecRelCheckGenVirtualNotNull() in
> particular looks like it could use a better location. Maybe it's
> better right after ExecConstraints, and ReportNotNullViolationError
> (or whatever we name it) can go after it.
I thought ExecRelCheckGenVirtualNotNull is very similar to ExecRelCheck,
that's why I put it close to ExecRelCheck.
putting it right after ExecConstraints is ok for me.
> * I don't find the name all_virtual_nns particularly appropriate. Maybe
> virtual_notnull_attrs?
>
in ATRewriteTable we already have "notnull_virtual_attrs"
we can rename it to notnull_virtual_attrs
> * There are some funny rules around nulls on rowtypes. I think allowing
> this tuple is wrong (and I left an XXX comment near where the 'argisrow'
> flag is set):
>
> create type twoints as (a int, b int);
> create table foo (a int, b int, c twoints generated always as (row(a,b)::twoints) not null);
> insert into foo values (null, null);
>
> I don't remember exactly what the rules are though so I may be wrong.
>
argisrow should set to false.
i think you mean the special value ``'(,)'``
create table t(a twoints not null);
insert into t select '(,)' returning a is null; --return true;
create table t1(a twoints not null generated always as ('(,)'::twoints));
insert into t1 default values returning a is null; --should return true.
i think you mean this thread:
https://postgr.es/m/173591158454.714.7664064332419606037@wrigleys.postgresql.org
should i put a test into generated_virtual.sql?
+ * We implement this by consing up a NullTest node for each virtual
trivial question.
I googled, and still found any explanation of the word "consing up".
Attachment
Hi,
response from ChatGPT, seems correct:
"Consing up" is an informal term derived from Lisp terminology. In this context, it means dynamically creating (allocating and constructing) a new NullTest node. Instead of reusing an existing node, the code allocates a fresh node—using PostgreSQL’s memory allocation (palloc) functions—and fills in its fields (like the generation expression, null test type, etc.). This new node is then prepared (compiled) for execution with ExecPrepareExpr and used to evaluate whether the generated expression returns a non-NULL value, thus enforcing the NOT NULL constraint on virtual generated columns.
+ * We implement this by consing up a NullTest node for each virtual
trivial question.
I googled, and still found any explanation of the word "consing up".
On 2025-Mar-20, jian he wrote:
> ATRewriteTable:
> for (i = 0; i < newTupDesc->natts; i++)
> {
> Form_pg_attribute attr = TupleDescAttr(newTupDesc, i);
> if (attr->attnotnull && !attr->attisdropped)
> {
> if (attr->attgenerated != ATTRIBUTE_GENERATED_VIRTUAL)
> notnull_attrs = lappend_int(notnull_attrs, i);
> else
> notnull_virtual_attrs = lappend_int(notnull_virtual_attrs,
> attr->attnum);
> }
> }
> this is kind of ugly? notnull_virtual_attrs is 1 based, notnull_attrs
> is 0 based.
> I want to change it all to 1 based. see v5-0002
Yeah, this inconsistency bothered me too. I have pushed your 0002 now,
which means your 0001 needs a small rebase. I'm going to leave 0001 for
Peter to commit, as it's mostly his turf.
> + * We implement this by consing up a NullTest node for each virtual
> trivial question.
> I googled, and still found any explanation of the word "consing up".
https://www.pc-freak.net/files/the-hacker-disctionary.html
You could change "cons up" to "manufacture" in that comment and get
about the same meaning. Maybe have a look at
git grep -C3 -E 'cons(|ing|ed) up'
I think the ChatGPT answer quoted by Zhou is roughly on point.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"El que vive para el futuro es un iluso, y el que vive para el pasado,
un imbécil" (Luis Adler, "Los tripulantes de la noche")
hi. rebase, and some minor code comments change.
Attachment
On 24.03.25 04:26, jian he wrote: > rebase, and some minor code comments change. I have committed this. I did a bit more editing on the naming of various things. In terms of functionality, in ExecConstraints() notnull_virtual_attrs and virtual_notnull_check were redundant, so I simplified that. I also cut back a little bit on the assertions. They were all ok, but I felt in some places they bloated the code quite a bit for no real gain.
On Fri, Mar 28, 2025 at 10:06 PM Peter Eisentraut <peter@eisentraut.org> wrote: > > On 24.03.25 04:26, jian he wrote: > > rebase, and some minor code comments change. > > I have committed this. > In an earlier thread, I also posted a patch for supporting virtual generated columns over domain type. The code is somehow similar, so I post the remaining patch to this thread. v7-0001 we need to compute the generation expression for the domain with constraints, thus rename ExecComputeStoredGenerated to ExecComputeGenerated. v7-0002 soft error variant of ExecPrepareExpr, ExecInitExpr. for soft error processing of CoerceToDomain. we don't want error messages like "value for domain d2 violates check constraint "d2_check"" while validating existing domain data, we want something like: +ERROR: column "b" of table "gtest24" contains values that violate the new constraint v7-0003 supports virtual generation columns over domain type. If the domain has constraints then we need to compute the virtual generation expression, that happens mainly within ExecComputeGenerated. if a virtual generation column type is domain_with_constraint, then ALTER DOMAIN ADD CONSTRAINTS need to revalidate these virtual generation expressions again. so in validateDomainCheckConstraint, validateDomainNotNullConstraint We need to fetch the generation expression (build_generation_expression), compile the generation expression (ExecPrepareExprSafe), and evaluate it (ExecEvalExprSwitchContext). I also posted patch summary earlier at [1] [1] https://postgr.es/m/CACJufxHT4R1oABzeQuvjb6DHrig7k-QSr6LEe53Q_nLi9pfanA@mail.gmail.com