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

  1. Suppress "may be used uninitialized" warnings from older compilers.

  2. Elide not-null constraint checks on child tables during PK creation

  3. Remove unnecessary code to handle CONSTR_NOTNULL

  4. Silence compilers about extractNotNullColumn()

  5. Add pg_constraint rows for not-null constraints