From 98ec192a4de85a90c595be47a03834d1a36abb16 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Tue, 8 Sep 2020 12:12:44 -0500 Subject: [PATCH v8 3/3] WIP: implement DROP INDEX CONCURRENTLY on partitioned index See also https://postgr.es/m/16594-d2956ca909585067@postgresql.org --- doc/src/sgml/ref/drop_index.sgml | 2 -- src/backend/catalog/dependency.c | 24 ++++++++++++++++++++++++ src/backend/catalog/index.c | 19 +------------------ src/backend/commands/tablecmds.c | 11 ----------- src/test/regress/expected/indexing.out | 4 +--- src/test/regress/sql/indexing.sql | 3 +-- 6 files changed, 27 insertions(+), 36 deletions(-) diff --git a/doc/src/sgml/ref/drop_index.sgml b/doc/src/sgml/ref/drop_index.sgml index 85cf23bca2..0aedd71bd6 100644 --- a/doc/src/sgml/ref/drop_index.sgml +++ b/doc/src/sgml/ref/drop_index.sgml @@ -57,8 +57,6 @@ DROP INDEX [ CONCURRENTLY ] [ IF EXISTS ] nameDROP INDEX commands can be performed within a transaction block, but DROP INDEX CONCURRENTLY cannot. - Lastly, indexes on partitioned tables cannot be dropped using this - option. For temporary tables, DROP INDEX is always diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index f515e2c308..25ad74da2d 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -395,6 +395,30 @@ performMultipleDeletions(const ObjectAddresses *objects, */ targetObjects = new_object_addresses(); + /* + * We must commit our transaction in order to make the first pg_index + * state update visible to other sessions. If the DROP machinery has + * already performed any other actions (removal of other objects, + * pg_depend entries, etc), the commit would make those actions + * permanent, which would leave us with inconsistent catalog state if + * we fail partway through the following sequence. Since DROP INDEX + * CONCURRENTLY is restricted to dropping just one index that has no + * dependencies, we should get here before anything's been done --- + * but let's check that to be sure. We can verify that the current + * transaction has not executed any transactional updates by checking + * that no XID has been assigned. + */ + if ((flags & PERFORM_DELETION_CONCURRENTLY) != 0 && + objects->numrefs == 1 && + objects->refs[0].classId == RelationRelationId && + get_rel_relkind(objects->refs[0].objectId) == RELKIND_PARTITIONED_INDEX) + { + if (GetTopTransactionIdIfAny() != InvalidTransactionId) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("DROP INDEX CONCURRENTLY must be first action in transaction"))); + } + for (i = 0; i < objects->numrefs; i++) { const ObjectAddress *thisobj = objects->refs + i; diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 0974f3e23a..5f250deeda 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2095,24 +2095,6 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode) */ if (concurrent) { - /* - * We must commit our transaction in order to make the first pg_index - * state update visible to other sessions. If the DROP machinery has - * already performed any other actions (removal of other objects, - * pg_depend entries, etc), the commit would make those actions - * permanent, which would leave us with inconsistent catalog state if - * we fail partway through the following sequence. Since DROP INDEX - * CONCURRENTLY is restricted to dropping just one index that has no - * dependencies, we should get here before anything's been done --- - * but let's check that to be sure. We can verify that the current - * transaction has not executed any transactional updates by checking - * that no XID has been assigned. - */ - if (GetTopTransactionIdIfAny() != InvalidTransactionId) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("DROP INDEX CONCURRENTLY must be first action in transaction"))); - /* * Mark index invalid by updating its pg_index entry */ @@ -2180,6 +2162,7 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode) */ CommitTransactionCommand(); StartTransactionCommand(); + PushActiveSnapshot(GetTransactionSnapshot()); /* * Wait till every transaction that saw the old index state has diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 16285ad09f..83092f8f6b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1374,17 +1374,6 @@ RemoveRelations(DropStmt *drop) flags |= PERFORM_DELETION_CONCURRENTLY; } - /* - * Concurrent index drop cannot be used with partitioned indexes, - * either. - */ - if ((flags & PERFORM_DELETION_CONCURRENTLY) != 0 && - get_rel_relkind(relOid) == RELKIND_PARTITIONED_INDEX) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot drop partitioned index \"%s\" concurrently", - rel->relname))); - /* OK, we're ready to delete this one */ obj.classId = RelationRelationId; obj.objectId = relOid; diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out index c6d7cfd446..18645dd9c6 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -226,9 +226,7 @@ create table idxpart1 partition of idxpart for values from (0) to (10); drop index idxpart1_a_idx; -- no way ERROR: cannot drop index idxpart1_a_idx because index idxpart_a_idx requires it HINT: You can drop index idxpart_a_idx instead. -drop index concurrently idxpart_a_idx; -- unsupported -ERROR: cannot drop partitioned index "idxpart_a_idx" concurrently -drop index idxpart_a_idx; -- both indexes go away +drop index concurrently idxpart_a_idx; select relname, relkind from pg_class where relname like 'idxpart%' order by relname; relname | relkind diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql index 3d4b6e9bc9..610c12de9e 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -104,8 +104,7 @@ create table idxpart (a int) partition by range (a); create index on idxpart (a); create table idxpart1 partition of idxpart for values from (0) to (10); drop index idxpart1_a_idx; -- no way -drop index concurrently idxpart_a_idx; -- unsupported -drop index idxpart_a_idx; -- both indexes go away +drop index concurrently idxpart_a_idx; select relname, relkind from pg_class where relname like 'idxpart%' order by relname; create index on idxpart (a); -- 2.17.0