Stop creating constraints during DETACH CONCURRENTLY

Commit 71f4c8c6f74b (which implemented DETACH CONCURRENTLY) added code
to create a separate table constraint when a table is detached
concurrently, identical to the partition constraint, on the theory that
such a constraint was needed in case the optimizer had constructed any
query plans that depended on the constraint being there.  However, that
theory was apparently bogus because any such plans would be invalidated.

For hash partitioning, those constraints are problematic, because their
expressions reference the OID of the parent partitioned table, to which
the detached table is no longer related; this causes all sorts of
problems (such as inability of restoring a pg_dump of that table, and
the table no longer working properly if the partitioned table is later
dropped).

We'd like to get rid of all those constraints.  In fact, for branch
master, do that -- no longer create any substitute constraints.
However, out of fear that some users might somehow depend on these
constraints for other partitioning strategies, for stable branches
(back to 14, which added DETACH CONCURRENTLY), only do it for hash
partitioning.

(If you repeatedly DETACH CONCURRENTLY and then ATTACH a partition, then
with this constraint addition you don't need to scan the table in the
ATTACH step, which presumably is good.  But if users really valued this
feature, they would have requested that it worked for non-concurrent
DETACH also.)

Author: Haiyang Li <mohen.lhy@alibaba-inc.com>
Reported-by: Fei Changhong <feichanghong@qq.com>
Reported-by: Haiyang Li <mohen.lhy@alibaba-inc.com>
Backpatch-through: 14
Discussion: https://postgr.es/m/18371-7fef49f63de13f02@postgresql.org
Discussion: https://postgr.es/m/19070-781326347ade7c57@postgresql.org

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 015e11af67bd8287380350cf04fc1581640cd691..09f8569d4e8247e1db68a1dda209e861f39b1d09 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -18096,13 +18096,14 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
        Relation        partRel;
        ObjectAddress address;
        Oid                     defaultPartOid;
+       PartitionDesc partdesc;
 
        /*
         * We must lock the default partition, because detaching this partition
         * will change its partition constraint.
         */
-       defaultPartOid =
-               get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, true));
+       partdesc = RelationGetPartitionDesc(rel, true);
+       defaultPartOid = get_default_oid_from_partdesc(partdesc);
        if (OidIsValid(defaultPartOid))
        {
                /*
@@ -18169,10 +18170,13 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
                char       *partrelname;
 
                /*
-                * Add a new constraint to the partition being detached, which
-                * supplants the partition constraint (unless there is one already).
+                * For strategies other than hash, add a constraint to the partition
+                * being detached which supplants the partition constraint. For hash
+                * we cannot do that, because the constraint would reference the
+                * partitioned table OID, possibly causing problems later.
                 */
-               DetachAddConstraintIfNeeded(wqueue, partRel);
+               if (partdesc->boundinfo->strategy != PARTITION_STRATEGY_HASH)
+                       DetachAddConstraintIfNeeded(wqueue, partRel);
 
                /*
                 * We're almost done now; the only traces that remain are the

diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index fcae98a7affa00bbff161482baabc1f0cbc20ea2..5cdf64e829b36eb01f00a9d3082751ce8a47baa1 100644 (file)
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4345,6 +4345,14 @@ Check constraints:
     "part_rp100_a_check" CHECK (a >= 123 AND a < 133 AND a IS NOT NULL)
 
 DROP TABLE range_parted2;
+-- Test that hash partitions continue to work after they're concurrently
+-- detached (bugs #18371, #19070)
+CREATE TABLE hash_parted2 (a int) PARTITION BY HASH(a);
+CREATE TABLE part_hp PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 2, REMAINDER 0);
+ALTER TABLE hash_parted2 DETACH PARTITION part_hp CONCURRENTLY;
+DROP TABLE hash_parted2;
+INSERT INTO part_hp VALUES (1);
+DROP TABLE part_hp;
 -- Check ALTER TABLE commands for partitioned tables and partitions
 -- cannot add/drop column to/from *only* the parent
 ALTER TABLE ONLY list_parted2 ADD COLUMN c int;

diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index fe59ff42546b0b5eed342662ff1786d42d72123f..a4c7dfadece256f7cb1fc3887885a20f5c351d51 100644 (file)
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2794,6 +2794,15 @@ ALTER TABLE range_parted2 DETACH PARTITION part_rp100 CONCURRENTLY;
 \d part_rp100
 DROP TABLE range_parted2;
 
+-- Test that hash partitions continue to work after they're concurrently
+-- detached (bugs #18371, #19070)
+CREATE TABLE hash_parted2 (a int) PARTITION BY HASH(a);
+CREATE TABLE part_hp PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 2, REMAINDER 0);
+ALTER TABLE hash_parted2 DETACH PARTITION part_hp CONCURRENTLY;
+DROP TABLE hash_parted2;
+INSERT INTO part_hp VALUES (1);
+DROP TABLE part_hp;
+
 -- Check ALTER TABLE commands for partitioned tables and partitions
 
 -- cannot add/drop column to/from *only* the parent