Skip to content

Commit 5bbdfa8

Browse files
michaelpqmichaelpq
committed
Fix waits of REINDEX CONCURRENTLY for indexes with predicates or expressions
As introduced by f9900df, a REINDEX CONCURRENTLY job done for an index with predicates or expressions would set PROC_IN_SAFE_IC in its MyProc->statusFlags, causing it to be ignored by other concurrent operations. Such concurrent index rebuilds should never be ignored, as a predicate or an expression could call a user-defined function that accesses a different table than the table where the index is rebuilt. A test that uses injection points is added, backpatched down to 17. Michail has proposed a different test, but I have added something simpler with more coverage. Oversight in f9900df. Author: Michail Nikolaev Discussion: https://postgr.es/m/CANtu0oj9A3kZVduFTG0vrmGnKB+DCHgEpzOp0qAyOgmks84j0w@mail.gmail.com Backpatch-through: 14
1 parent dd8bea8 commit 5bbdfa8

File tree

5 files changed

+92
-3
lines changed

5 files changed

+92
-3
lines changed

‎src/backend/commands/indexcmds.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
#include "utils/builtins.h"
6262
#include "utils/fmgroids.h"
6363
#include "utils/guc.h"
64+
#include "utils/injection_point.h"
6465
#include "utils/inval.h"
6566
#include "utils/lsyscache.h"
6667
#include "utils/memutils.h"
@@ -3782,8 +3783,16 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
37823783
RestrictSearchPath();
37833784

37843785
/* determine safety of this index for set_indexsafe_procflags */
3785-
idx->safe = (indexRel->rd_indexprs == NIL &&
3786-
indexRel->rd_indpred == NIL);
3786+
idx->safe = (RelationGetIndexExpressions(indexRel) == NIL &&
3787+
RelationGetIndexPredicate(indexRel) == NIL);
3788+
3789+
#ifdef USE_INJECTION_POINTS
3790+
if (idx->safe)
3791+
INJECTION_POINT("reindex-conc-index-safe");
3792+
else
3793+
INJECTION_POINT("reindex-conc-index-not-safe");
3794+
#endif
3795+
37873796
idx->tableId = RelationGetRelid(heapRel);
37883797
idx->amId = indexRel->rd_rel->relam;
37893798

‎src/test/modules/injection_points/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ EXTENSION = injection_points
1010
DATA = injection_points--1.0.sql
1111
PGFILEDESC = "injection_points - facility for injection points"
1212

13-
REGRESS = injection_points
13+
REGRESS = injection_points reindex_conc
1414
REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
1515

1616
ISOLATION = inplace

‎src/test/modules/injection_points/expected/reindex_conc.out

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
-- Tests for REINDEX CONCURRENTLY
2+
CREATE EXTENSION injection_points;
3+
-- Check safety of indexes with predicates and expressions.
4+
SELECT injection_points_set_local();
5+
injection_points_set_local
6+
----------------------------
7+
8+
(1 row)
9+
10+
SELECT injection_points_attach('reindex-conc-index-safe', 'notice');
11+
injection_points_attach
12+
-------------------------
13+
14+
(1 row)
15+
16+
SELECT injection_points_attach('reindex-conc-index-not-safe', 'notice');
17+
injection_points_attach
18+
-------------------------
19+
20+
(1 row)
21+
22+
CREATE SCHEMA reindex_inj;
23+
CREATE TABLE reindex_inj.tbl(i int primary key, updated_at timestamp);
24+
CREATE UNIQUE INDEX ind_simple ON reindex_inj.tbl(i);
25+
CREATE UNIQUE INDEX ind_expr ON reindex_inj.tbl(ABS(i));
26+
CREATE UNIQUE INDEX ind_pred ON reindex_inj.tbl(i) WHERE mod(i, 2) = 0;
27+
CREATE UNIQUE INDEX ind_expr_pred ON reindex_inj.tbl(abs(i)) WHERE mod(i, 2) = 0;
28+
REINDEX INDEX CONCURRENTLY reindex_inj.ind_simple;
29+
NOTICE: notice triggered for injection point reindex-conc-index-safe
30+
REINDEX INDEX CONCURRENTLY reindex_inj.ind_expr;
31+
NOTICE: notice triggered for injection point reindex-conc-index-not-safe
32+
REINDEX INDEX CONCURRENTLY reindex_inj.ind_pred;
33+
NOTICE: notice triggered for injection point reindex-conc-index-not-safe
34+
REINDEX INDEX CONCURRENTLY reindex_inj.ind_expr_pred;
35+
NOTICE: notice triggered for injection point reindex-conc-index-not-safe
36+
-- Cleanup
37+
SELECT injection_points_detach('reindex-conc-index-safe');
38+
injection_points_detach
39+
-------------------------
40+
41+
(1 row)
42+
43+
SELECT injection_points_detach('reindex-conc-index-not-safe');
44+
injection_points_detach
45+
-------------------------
46+
47+
(1 row)
48+
49+
DROP TABLE reindex_inj.tbl;
50+
DROP SCHEMA reindex_inj;
51+
DROP EXTENSION injection_points;

‎src/test/modules/injection_points/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ tests += {
3434
'regress': {
3535
'sql': [
3636
'injection_points',
37+
'reindex_conc',
3738
],
3839
'regress_args': ['--dlpath', meson.build_root() / 'src/test/regress'],
3940
# The injection points are cluster-wide, so disable installcheck

‎src/test/modules/injection_points/sql/reindex_conc.sql

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
-- Tests for REINDEX CONCURRENTLY
2+
CREATE EXTENSION injection_points;
3+
4+
-- Check safety of indexes with predicates and expressions.
5+
SELECT injection_points_set_local();
6+
SELECT injection_points_attach('reindex-conc-index-safe', 'notice');
7+
SELECT injection_points_attach('reindex-conc-index-not-safe', 'notice');
8+
9+
CREATE SCHEMA reindex_inj;
10+
CREATE TABLE reindex_inj.tbl(i int primary key, updated_at timestamp);
11+
12+
CREATE UNIQUE INDEX ind_simple ON reindex_inj.tbl(i);
13+
CREATE UNIQUE INDEX ind_expr ON reindex_inj.tbl(ABS(i));
14+
CREATE UNIQUE INDEX ind_pred ON reindex_inj.tbl(i) WHERE mod(i, 2) = 0;
15+
CREATE UNIQUE INDEX ind_expr_pred ON reindex_inj.tbl(abs(i)) WHERE mod(i, 2) = 0;
16+
17+
REINDEX INDEX CONCURRENTLY reindex_inj.ind_simple;
18+
REINDEX INDEX CONCURRENTLY reindex_inj.ind_expr;
19+
REINDEX INDEX CONCURRENTLY reindex_inj.ind_pred;
20+
REINDEX INDEX CONCURRENTLY reindex_inj.ind_expr_pred;
21+
22+
-- Cleanup
23+
SELECT injection_points_detach('reindex-conc-index-safe');
24+
SELECT injection_points_detach('reindex-conc-index-not-safe');
25+
DROP TABLE reindex_inj.tbl;
26+
DROP SCHEMA reindex_inj;
27+
28+
DROP EXTENSION injection_points;

0 commit comments

Comments
 (0)