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; }
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.
