From 81e34efd77bc645041795c4e40b2bad9300d9d52 Mon Sep 17 00:00:00 2001 From: jian he Date: Fri, 8 Nov 2024 14:40:48 +0800 Subject: [PATCH v11 1/1] refactor AddRelationNotNullConstraints in AddRelationNotNullConstraints, first do sanity check then another loop to call StoreRelNotNull. --- src/backend/catalog/heap.c | 78 ++++++++++++++++++++++++++------------ 1 file changed, 53 insertions(+), 25 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 003af4bf21..f5cf5192ae 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2800,6 +2800,12 @@ AddRelationNotNullConstraints(Relation rel, List *constraints, List *givennames; List *nnnames; List *nncols = NIL; + List *inhcounts = NIL; + List *no_inherit_status = NIL; + ListCell *l1; + ListCell *l2; + ListCell *l3; + ListCell *l4; /* * We track two lists of names: nnnames keeps all the constraint names, @@ -2810,28 +2816,12 @@ AddRelationNotNullConstraints(Relation rel, List *constraints, nnnames = NIL; givennames = NIL; - /* - * First, create all not-null constraints that are directly specified by - * the user. Note that inheritance might have given us another source for - * each, so we must scan the old_notnulls list and increment inhcount for - * each element with identical attnum. We delete from there any element - * that we process. - * - * We don't use foreach() here because we have two nested loops over the - * constraint list, with possible element deletions in the inner one. If - * we used foreach_delete_current() it could only fix up the state of one - * of the loops, so it seems cleaner to use looping over list indexes for - * both loops. Note that any deletion will happen beyond where the outer - * loop is, so its index never needs adjustment. - */ - for (int outerpos = 0; outerpos < list_length(constraints); outerpos++) + for (int i = 0; i < list_length(constraints); i++) { Constraint *constr; AttrNumber attnum; - char *conname; - int inhcount = 0; - constr = list_nth_node(Constraint, constraints, outerpos); + constr = list_nth_node(Constraint, constraints, i); Assert(constr->contype == CONSTR_NOTNULL); @@ -2849,6 +2839,31 @@ AddRelationNotNullConstraints(Relation rel, List *constraints, errmsg("cannot add not-null constraint on system column \"%s\"", strVal(linitial(constr->keys)))); + nncols = list_append_unique_int(nncols, attnum); + } + + /* + * First, create all not-null constraints that are directly specified by + * the user. Note that inheritance might have given us another source for + * each, so we must scan the old_notnulls list and increment inhcount for + * each element with identical attnum. We delete from there any element + * that we process. + * + * We don't use foreach() here because we have two nested loops over the + * constraint list, with possible element deletions in the inner one. If + * we used foreach_delete_current() it could only fix up the state of one + * of the loops, so it seems cleaner to use looping over list indexes for + * both loops. Note that any deletion will happen beyond where the outer + * loop is, so its index never needs adjustment. + */ + for (int outerpos = 0; outerpos < list_length(constraints); outerpos++) + { + Constraint *constr; + char *conname; + int inhcount = 0; + + constr = list_nth_node(Constraint, constraints, outerpos); + /* * A column can only have one not-null constraint, so discard any * additional ones that appear for columns we already saw; but check @@ -2900,7 +2915,7 @@ AddRelationNotNullConstraints(Relation rel, List *constraints, */ foreach_ptr(CookedConstraint, old, old_notnulls) { - if (old->attnum == attnum) + if (old->attnum == list_nth_int(nncols, outerpos)) { /* * If we get a constraint from the parent, having a local NO @@ -2941,17 +2956,30 @@ AddRelationNotNullConstraints(Relation rel, List *constraints, else conname = ChooseConstraintName(RelationGetRelationName(rel), get_attname(RelationGetRelid(rel), - attnum, false), + list_nth_int(nncols, outerpos), false), "not_null", RelationGetNamespace(rel), nnnames); nnnames = lappend(nnnames, conname); + inhcounts = lappend_int(inhcounts, inhcount); + no_inherit_status = lappend_int(no_inherit_status, constr->is_no_inherit ? 1 : 0); + } - StoreRelNotNull(rel, conname, + Assert(list_length(nncols)== list_length(inhcounts)); + Assert(list_length(no_inherit_status)== list_length(inhcounts)); + Assert(list_length(inhcounts)== list_length(nnnames)); + + forfour(l1, nnnames, l2, nncols, l3, inhcounts, l4, no_inherit_status) + { + bool is_no_inherit; + char *the_conname = (char *) lfirst(l1); + int32 attnum = lfirst_int(l2); + int32 inhcount = lfirst_int(l3); + is_no_inherit = (lfirst_int(l4) == 1) ? true : false; + + StoreRelNotNull(rel, the_conname, attnum, true, true, - inhcount, constr->is_no_inherit); - - nncols = lappend_int(nncols, attnum); + inhcount, is_no_inherit); } /* @@ -3027,7 +3055,7 @@ AddRelationNotNullConstraints(Relation rel, List *constraints, /* ignore the origin constraint's is_local and inhcount */ StoreRelNotNull(rel, conname, cooked->attnum, true, - false, inhcount, false); + false, inhcount, cooked->is_no_inherit); nncols = lappend_int(nncols, cooked->attnum); } -- 2.34.1