Feedback-ID: ia2694551:Fastmail
Date: Tue, 5 Nov 2024 13:01:12 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Robert Haas <robertmhaas@gmail.com>, 
	Amit Langote <amitlangote09@gmail.com>
Subject: doc fail about ALTER TABLE ATTACH re. NO INHERIT
Message-ID: <202411051201.zody6mld7vkw@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/202411051201.zody6mld7vkw%40alvherre.pgsql>
Precedence: bulk

Hello,

While doing final review for not-null constraints, I noticed that the
ALTER TABLE ATTACH PARTITION have this phrase:

    If any of the CHECK constraints of the table being attached are marked NO
    INHERIT, the command will fail; such constraints must be recreated without the
    NO INHERIT clause.

However, this is not true and apparently has never been true.  I tried
this in both master and pg10:

create table parted (a int) partition by list (a);
create table part1 (a int , check (a > 0) no inherit);
alter table parted attach partition part1 for values in (1);

In both versions (and I imagine all intermediate ones) that sequence
works fine and results in this table:

                                   Table "public.part1"
 Column │  Type   │ Collation │ Nullable │ Default │ Storage │ Stats target │ Description 
────────┼─────────┼───────────┼──────────┼─────────┼─────────┼──────────────┼─────────────
 a      │ integer │           │          │         │ plain   │              │ 
Partition of: parted FOR VALUES IN (1)
Partition constraint: ((a IS NOT NULL) AND (a = 1))
Check constraints:
    "part1_a_check" CHECK (a > 0) NO INHERIT

On the other hand, if we were to throw an error in the ALTER TABLE as
the docs say, it would serve no purpose: the partition cannot have any
more descendants, so the fact that the CHECK constraint is NO INHERIT
makes no difference.  So I think the code is fine and we should just fix
the docs.


Note that if you interpret it the other way around, i.e., that the
"table being attached" is the parent table, then the first CREATE
already fails:

create table parted2 (a int check (a > 0) no inherit) partition by list (a);
ERROR:  cannot add NO INHERIT constraint to partitioned table "parted2"

This says that we don't need to worry about this condition in the parent
table either.

All in all, I think this text serves no purpose and should be removed
(from all live branches), as in the attached patch.

This text came in with the original partitioning commit f0e44751d717.
CCing Robert and Amit.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"No renuncies a nada. No te aferres a nada."