Hi all, Some of the feedback I got after f24a08ec13eb involved the fact that indexes could also be dropped during the phase 1 of REINDEX CONCURRENTLY when the expressions or predicates are evaluated. The damage is limited to cache lookup failures or assertion hits during phase 2, thanks to the protections that f24a08ec13eb has added.
Anyway, it would be nice to fix this issue. Attached is a patch addressing the problem with a test, for a backpatch down to v15. Thoughts or objections? -- Michael
From 67a2c319cc3f83b670241f5440fdbab4ccb8ccfa Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Sat, 14 Nov 2026 09:20:12 +0900 Subject: [PATCH] Pin relations during phase 1 of REINDEX CONCURRENTLY Phase 1 of ReindexRelationConcurrently() (creation of the new indexes) evaluates index expressions and predicates when computing the safety of each index and when creating the definition of each new index. These may call functions run as the table owner. The old and new indexes processed in the previous iterations of the loop were closed at this stage. Hence, a function was able to drop one of them or reindex one of the new indexes, leading to an assertion failure in the build phase, or to an internal error due to a dropped index. The relations opened in phase 1 are now kept pinned until the session locks are taken, so as DDLs on them are blocked by CheckTableNotInUse(). The build and validation phases were already pinning the relations. Author: Michael Paquier <[email protected]> Discussion: https://postgr.es/m/TODO Backpatch-through: 15 --- src/backend/commands/indexcmds.c | 16 ++++++++-- src/test/regress/expected/create_index.out | 37 +++++++++++++++++++++- src/test/regress/sql/create_index.sql | 26 ++++++++++++++- 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 68fc87b76a0e..8414b8037222 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -3790,6 +3790,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein List *indexIds = NIL; List *newIndexIds = NIL; List *relationLocks = NIL; + List *phase1Rels = NIL; List *lockTags = NIL; ListCell *lc, *lc2; @@ -4215,8 +4216,13 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein MemoryContextSwitchTo(oldcontext); - index_close(indexRel, NoLock); - index_close(newIndexRel, NoLock); + /* + * Index expressions and predicates of the indexes processed in the + * next iterations are evaluated in this loop, so keep the relations + * opened until all the new indexes are created. + */ + phase1Rels = lappend(phase1Rels, indexRel); + phase1Rels = lappend(phase1Rels, newIndexRel); /* Roll back any GUC changes executed by index functions */ AtEOXact_GUC(false, save_nestlevel); @@ -4224,7 +4230,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein /* Restore userid and security context */ SetUserIdAndSecContext(save_userid, save_sec_context); - table_close(heapRel, NoLock); + phase1Rels = lappend(phase1Rels, heapRel); /* * If a statement is available, telling that this comes from a REINDEX @@ -4279,6 +4285,10 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein LockRelationIdForSession(lockrelid, ShareUpdateExclusiveLock); } + /* Session locks are taken, release the relations opened in phase 1 */ + ReindexUnpinRelations(phase1Rels); + phase1Rels = NIL; + PopActiveSnapshot(); CommitTransactionCommand(); StartTransactionCommand(); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e914c809b820..857603cd2b4f 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2878,7 +2878,8 @@ Indexes: DROP TABLE concur_appclass_tab; -- Check that a function called from an index expression is not able to --- manipulate a relation involved in the operation. +-- manipulate a relation involved in the operation. This involves phase 2 of +-- REINDEX CONCURRENTLY. CREATE TABLE concur_reindex_ddl_tab (i int); INSERT INTO concur_reindex_ddl_tab VALUES (1); -- Use two functions, one volatile called in a second, immutable, one. @@ -2901,6 +2902,40 @@ REINDEX INDEX CONCURRENTLY concur_reindex_ddl_ind2; -- ok REINDEX INDEX CONCURRENTLY concur_reindex_ddl_ind1; -- ok REINDEX INDEX CONCURRENTLY concur_reindex_ddl_ind2; -- dropped ERROR: relation "concur_reindex_ddl_ind2" does not exist +DROP TABLE concur_reindex_ddl_tab; +DROP FUNCTION concur_reindex_ddl_im(int); +DROP FUNCTION concur_reindex_ddl_f(); +-- Same, with the function called when the index definitions are created. +-- This involves phase 1 of REINDEX CONCURRENTLY. +CREATE TABLE concur_reindex_ddl_tab (i int); +INSERT INTO concur_reindex_ddl_tab VALUES (1); +CREATE FUNCTION concur_reindex_ddl_f() RETURNS void LANGUAGE plpgsql AS + 'begin end'; +CREATE FUNCTION concur_reindex_ddl_im(int) RETURNS int + LANGUAGE plpgsql IMMUTABLE AS + 'begin perform public.concur_reindex_ddl_f(); return $1; end'; +CREATE INDEX concur_reindex_ddl_ind1 ON concur_reindex_ddl_tab (i); +CREATE INDEX concur_reindex_ddl_ind2 ON + concur_reindex_ddl_tab ((i + concur_reindex_ddl_im(0))); +CREATE OR REPLACE FUNCTION concur_reindex_ddl_f() RETURNS void + LANGUAGE plpgsql AS + 'begin DROP INDEX public.concur_reindex_ddl_ind1; end'; +REINDEX TABLE CONCURRENTLY concur_reindex_ddl_tab; -- error +ERROR: cannot DROP INDEX "concur_reindex_ddl_ind1" because it is being used by active queries in this session +CONTEXT: SQL statement "DROP INDEX public.concur_reindex_ddl_ind1" +PL/pgSQL function concur_reindex_ddl_f() line 1 at SQL statement +SQL statement "SELECT public.concur_reindex_ddl_f()" +PL/pgSQL function concur_reindex_ddl_im(integer) line 1 at PERFORM +SELECT c.relname, i.indisvalid FROM pg_index i + JOIN pg_class c ON c.oid = i.indexrelid + WHERE i.indrelid = 'concur_reindex_ddl_tab'::regclass + ORDER BY c.relname COLLATE "C"; + relname | indisvalid +-------------------------+------------ + concur_reindex_ddl_ind1 | t + concur_reindex_ddl_ind2 | t +(2 rows) + DROP TABLE concur_reindex_ddl_tab; DROP FUNCTION concur_reindex_ddl_im(int); DROP FUNCTION concur_reindex_ddl_f(); diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index e4b8cee1e1c2..27e038fa8087 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -1144,7 +1144,8 @@ REINDEX TABLE CONCURRENTLY concur_appclass_tab; DROP TABLE concur_appclass_tab; -- Check that a function called from an index expression is not able to --- manipulate a relation involved in the operation. +-- manipulate a relation involved in the operation. This involves phase 2 of +-- REINDEX CONCURRENTLY. CREATE TABLE concur_reindex_ddl_tab (i int); INSERT INTO concur_reindex_ddl_tab VALUES (1); -- Use two functions, one volatile called in a second, immutable, one. @@ -1163,6 +1164,29 @@ REINDEX INDEX CONCURRENTLY concur_reindex_ddl_ind2; -- dropped DROP TABLE concur_reindex_ddl_tab; DROP FUNCTION concur_reindex_ddl_im(int); DROP FUNCTION concur_reindex_ddl_f(); +-- Same, with the function called when the index definitions are created. +-- This involves phase 1 of REINDEX CONCURRENTLY. +CREATE TABLE concur_reindex_ddl_tab (i int); +INSERT INTO concur_reindex_ddl_tab VALUES (1); +CREATE FUNCTION concur_reindex_ddl_f() RETURNS void LANGUAGE plpgsql AS + 'begin end'; +CREATE FUNCTION concur_reindex_ddl_im(int) RETURNS int + LANGUAGE plpgsql IMMUTABLE AS + 'begin perform public.concur_reindex_ddl_f(); return $1; end'; +CREATE INDEX concur_reindex_ddl_ind1 ON concur_reindex_ddl_tab (i); +CREATE INDEX concur_reindex_ddl_ind2 ON + concur_reindex_ddl_tab ((i + concur_reindex_ddl_im(0))); +CREATE OR REPLACE FUNCTION concur_reindex_ddl_f() RETURNS void + LANGUAGE plpgsql AS + 'begin DROP INDEX public.concur_reindex_ddl_ind1; end'; +REINDEX TABLE CONCURRENTLY concur_reindex_ddl_tab; -- error +SELECT c.relname, i.indisvalid FROM pg_index i + JOIN pg_class c ON c.oid = i.indexrelid + WHERE i.indrelid = 'concur_reindex_ddl_tab'::regclass + ORDER BY c.relname COLLATE "C"; +DROP TABLE concur_reindex_ddl_tab; +DROP FUNCTION concur_reindex_ddl_im(int); +DROP FUNCTION concur_reindex_ddl_f(); -- Partitions -- Create some partitioned tables -- 2.56.0
signature.asc
Description: PGP signature
