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/
