Re: not null constraints, again
Alvaro Herrera <alvherre@alvh.no-ip.org>
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: jian he <jian.universality@gmail.com>
Cc: Tender Wang <tndrwang@gmail.com>, Pg Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-09-24T10:51:54Z
Lists: pgsql-hackers
On 2024-Sep-24, jian he wrote:
> static void
> set_attnotnull(List **wqueue, Relation rel, AttrNumber attnum, bool recurse,
> LOCKMODE lockmode)
> {
> What do you think of the above refactor?
> (I intentionally deleted empty new line)
Looks nicer ... but you know what? After spending some more time with
it, I realized that one caller is dead code (in
AttachPartitionEnsureIndexes) and another caller doesn't need to ask for
recursion, because it recurses itself (in ATAddCheckNNConstraint). So
that leaves us with a grand total of zero callers that need the
recursion here ... which means we can simplify it to the case that it
only examines a single relation and never recurses.
So I've stripped it down to its bare minimum:
/*
* Helper to set pg_attribute.attnotnull if it isn't set, and to tell phase 3
* to verify it.
*
* When called to alter an existing table, 'wqueue' must be given so that we
* can queue a check that existing tuples pass the constraint. When called
* from table creation, 'wqueue' should be passed as NULL.
*/
static void
set_attnotnull(List **wqueue, Relation rel, AttrNumber attnum,
LOCKMODE lockmode)
{
Oid reloid = RelationGetRelid(rel);
HeapTuple tuple;
Form_pg_attribute attForm;
CheckAlterTableIsSafe(rel);
tuple = SearchSysCacheCopyAttNum(reloid, attnum);
if (!HeapTupleIsValid(tuple))
elog(ERROR, "cache lookup failed for attribute %d of relation %u",
attnum, reloid);
attForm = (Form_pg_attribute) GETSTRUCT(tuple);
if (!attForm->attnotnull)
{
Relation attr_rel;
attr_rel = table_open(AttributeRelationId, RowExclusiveLock);
attForm->attnotnull = true;
CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple);
if (wqueue && !NotNullImpliedByRelConstraints(rel, attForm))
{
AlteredTableInfo *tab;
tab = ATGetQueueEntry(wqueue, rel);
tab->verify_new_notnull = true;
}
CommandCounterIncrement();
table_close(attr_rel, RowExclusiveLock);
}
heap_freetuple(tuple);
}
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
Commits
-
Suppress "may be used uninitialized" warnings from older compilers.
- fc5e966f73f0 18.0 landed
-
Elide not-null constraint checks on child tables during PK creation
- 11ff192b5bb7 18.0 landed
-
Remove unnecessary code to handle CONSTR_NOTNULL
- 5b291d1c9c09 18.0 landed
-
Silence compilers about extractNotNullColumn()
- ff239c3bf4e8 18.0 landed
-
Add pg_constraint rows for not-null constraints
- 14e87ffa5c54 18.0 landed