Odd behavior of partitioned ALTER TABLE ONLY ... ADD PRIMARY KEY - Mailing list pgsql-hackers

From Tom Lane
Subject Odd behavior of partitioned ALTER TABLE ONLY ... ADD PRIMARY KEY
Date
Msg-id 16115.1555874162@sss.pgh.pa.us
Whole thread Raw
Responses Re: Odd behavior of partitioned ALTER TABLE ONLY ... ADD PRIMARY KEY
List pgsql-hackers
While fooling around with the patch shown at
<12166.1555559689@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



pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: Re: [PATCH v1] Add \echo_stderr to psql
Next
From: Fabien COELHO
Date:
Subject: Re: [PATCH v1] Add \echo_stderr to psql