From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Amit Langote <amitlangote09@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY
Date: Mon, 15 Mar 2021 20:04:37 -0300
Message-ID: <20210315230437.GA20801@alvherre.pgsql> (raw)
In-Reply-To: <20210226203236.GA30377@alvherre.pgsql>
On 2021-Feb-26, Alvaro Herrera wrote:
> Hmm, but if we take this approach, then we're still vulnerable to the
> problem that somebody can do DETACH CONCURRENTLY and cancel the wait (or
> crash the server), then mess up the state before doing DETACH FINALIZE:
> when they cancel the wait, the lock will be released.
>
> I think the right fix is to disallow any action on a partition which is
> pending detach other than DETACH FINALIZE. (Didn't do that here.)
Here's a fixup patch to do it that way. I tried running the commands
you showed and one of them immediately dies with the new error message;
I can't cause the bogus constraint to show up anymore.
I'll clean this up for a real submission tomorrow.
--
Álvaro Herrera 39°49'30"S 73°17'W
"The Gord often wonders why people threaten never to come back after they've
been told never to return" (www.actsofgord.com)
diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.cindex 3fd0880ca0..83093f9730 100644--- a/src/backend/catalog/pg_inherits.c+++ b/src/backend/catalog/pg_inherits.c@@ -535,3 +535,39 @@ DeleteInheritsTuple(Oid inhrelid, Oid inhparent, bool detached,
return found;
}
++bool+PartitionHasPendingDetach(Oid partoid)+{+ Relation catalogRelation;+ ScanKeyData key;+ SysScanDesc scan;+ HeapTuple inheritsTuple;++ /*+ * Find pg_inherits entries by inhrelid.+ */+ catalogRelation = table_open(InheritsRelationId, RowExclusiveLock);+ ScanKeyInit(&key,+ Anum_pg_inherits_inhrelid,+ BTEqualStrategyNumber, F_OIDEQ,+ ObjectIdGetDatum(partoid));+ scan = systable_beginscan(catalogRelation, InheritsRelidSeqnoIndexId,+ true, NULL, 1, &key);++ while (HeapTupleIsValid(inheritsTuple = systable_getnext(scan)))+ {+ bool detached;++ detached = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhdetached;++ /* Done */+ systable_endscan(scan);+ table_close(catalogRelation, RowExclusiveLock);++ return detached;+ }++ elog(ERROR, "relation %u is not a partition", partoid);+ return false; /* keep compiler quiet */+}diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 5986b4dd56..5074c0ff2b 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -17106,6 +17106,8 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
*/
partRel = table_openrv(name, concurrent ? ShareLock :
AccessExclusiveLock);
+ if (PartitionHasPendingDetach(RelationGetRelid(partRel)))+ elog(ERROR, "hey, don't!");
/*
* Check inheritance conditions and either delete the pg_inherits row
diff --git a/src/include/catalog/pg_inherits.h b/src/include/catalog/pg_inherits.hindex 155596907c..19243ea9e4 100644--- a/src/include/catalog/pg_inherits.h+++ b/src/include/catalog/pg_inherits.h@@ -61,5 +61,6 @@ extern void StoreSingleInheritance(Oid relationId, Oid parentOid,
int32 seqNumber);
extern bool DeleteInheritsTuple(Oid inhrelid, Oid inhparent, bool allow_detached,
const char *childname);
+extern bool PartitionHasPendingDetach(Oid partoid);
#endif /* PG_INHERITS_H */
Attachments:
[text/plain] dont.notpatch (2.2K, ../20210315230437.GA20801@alvherre.pgsql/2-dont.notpatch)
download | inline diff:diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.cindex 3fd0880ca0..83093f9730 100644--- a/src/backend/catalog/pg_inherits.c+++ b/src/backend/catalog/pg_inherits.c@@ -535,3 +535,39 @@ DeleteInheritsTuple(Oid inhrelid, Oid inhparent, bool detached,
return found;
}
++bool+PartitionHasPendingDetach(Oid partoid)+{+ Relation catalogRelation;+ ScanKeyData key;+ SysScanDesc scan;+ HeapTuple inheritsTuple;++ /*+ * Find pg_inherits entries by inhrelid.+ */+ catalogRelation = table_open(InheritsRelationId, RowExclusiveLock);+ ScanKeyInit(&key,+ Anum_pg_inherits_inhrelid,+ BTEqualStrategyNumber, F_OIDEQ,+ ObjectIdGetDatum(partoid));+ scan = systable_beginscan(catalogRelation, InheritsRelidSeqnoIndexId,+ true, NULL, 1, &key);++ while (HeapTupleIsValid(inheritsTuple = systable_getnext(scan)))+ {+ bool detached;++ detached = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhdetached;++ /* Done */+ systable_endscan(scan);+ table_close(catalogRelation, RowExclusiveLock);++ return detached;+ }++ elog(ERROR, "relation %u is not a partition", partoid);+ return false; /* keep compiler quiet */+}diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 5986b4dd56..5074c0ff2b 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -17106,6 +17106,8 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
*/
partRel = table_openrv(name, concurrent ? ShareLock :
AccessExclusiveLock);
+ if (PartitionHasPendingDetach(RelationGetRelid(partRel)))+ elog(ERROR, "hey, don't!");
/*
* Check inheritance conditions and either delete the pg_inherits row
diff --git a/src/include/catalog/pg_inherits.h b/src/include/catalog/pg_inherits.hindex 155596907c..19243ea9e4 100644--- a/src/include/catalog/pg_inherits.h+++ b/src/include/catalog/pg_inherits.h@@ -61,5 +61,6 @@ extern void StoreSingleInheritance(Oid relationId, Oid parentOid,
int32 seqNumber);
extern bool DeleteInheritsTuple(Oid inhrelid, Oid inhparent, bool allow_detached,
const char *childname);
+extern bool PartitionHasPendingDetach(Oid partoid);
#endif /* PG_INHERITS_H */
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, pryzby@telsasoft.com, amitlangote09@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY
In-Reply-To: <20210315230437.GA20801@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