While fooling around with the patch shown at
<12166.1555559...@sss.pgh.pa.us>, I noticed this rather strange
pre-existing behavior (tested on v11 as well as HEAD):
regression=# create table idxpart (a int) partition by range (a);
CREATE TABLE
regression=# create table idxpart0 (like idxpart);
CREATE TABLE
regression=# alter table idxpart0 add unique(a);
ALTER TABLE
regression=# alter table idxpart attach partition idxpart0 default;
ALTER TABLE
regression=# \d idxpart0
Table "public.idxpart0"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
Partition of: idxpart DEFAULT
Indexes:
"idxpart0_a_key" UNIQUE CONSTRAINT, btree (a)
regression=# alter table only idxpart add primary key (a);
ALTER TABLE
regression=# \d idxpart0
Table "public.idxpart0"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | not null |
Partition of: idxpart DEFAULT
Indexes:
"idxpart0_a_key" UNIQUE CONSTRAINT, btree (a)
In other words, even though I said ALTER TABLE ONLY, this command went
and modified the partition to have a not null constraint that it
didn't have before. (And yes, it scans the partition to verify that.)
ISTM that this is a bug, not a feature: if there's any point at
all to saying ONLY in this context, it's that we're not supposed
to be doing anything as expensive as adding a new constraint to
a child partition. No? So I think that this should have failed.
We need to require the partition(s) to already have attnotnull set.
Does anyone want to argue differently?
Note that if we ever get around to tracking the inheritance status
of attnotnull, this'd also require incrementing an inheritance counter
for attnotnull, meaning we'd need a stronger lock on the partition
than is needed just to check not-nullness. But we already need to
increment pg_constraint.coninhcount for the child's unique or pkey
constraint that we're co-opting, so that doesn't sound like a real
problem. (In practice it seems that this command takes AEL on
the child partition, which surprises me; shouldn't we be striving
for something less?)
regards, tom lane
