From: | ego alter <xunengzhou(at)gmail(dot)com> |
---|---|
To: | jian he <jian(dot)universality(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: support virtual generated column not null constraint |
Date: | 2025-02-26 07:00:48 |
Message-ID: | CABPTF7Vhmy-qGjB5XvpO-BXbf9bCR7BO5J548XzD2Bx7xOdkBg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
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(dot)universality(at)gmail(dot)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.
>
From | Date | Subject | |
---|---|---|---|
Next Message | John Naylor | 2025-02-26 07:04:38 | Re: Doc fix of aggressive vacuum threshold for multixact members storage |
Previous Message | Maxim Orlov | 2025-02-26 06:57:45 | Re: Spinlock can be released twice in procsignal.c |