From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: jian he <jian.universality@gmail.com>
Cc: Tender Wang <tndrwang@gmail.com>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: not null constraints, again
Date: Tue, 24 Sep 2024 12:51:54 +0200
Message-ID: <202409241051.xhdeeqazf63f@alvherre.pgsql> (raw)
In-Reply-To: <CACJufxGgO8QDt4mwT7PksskpFK-sFbJdNvqBcD4+KaQGu2OTZw@mail.gmail.com>
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/
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, jian.universality@gmail.com, tndrwang@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: not null constraints, again
In-Reply-To: <202409241051.xhdeeqazf63f@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox