On Fri, Sep 06, 2024 at 01:27:12PM +0200, Michail Nikolaev wrote: > While working on [1], I have found a small issue with correctness > of set_indexsafe_procflags usage in ReindexRelationConcurrently introduced > in [2]. > > > idx->safe = (indexRel->rd_indexprs == NIL && indexRel->rd_indpred == NIL); > > It is always true because there are no RelationGetIndexExpressions > and RelationGetIndexPredicate before that check. > > Two patches with reproducer + fix are attached. > > The issue is simple, but I'll register this in commitfest just in case.
Ugh. It means that we've always considered as "safe" concurrent rebuilds of indexes that have expressions or predicates, but they're not safe at all. Other concurrent jobs should wait for them. Adding these two calls as you are suggesting is probably a good idea anyway to force a correct setup of the flag. Will see about that. I don't understand why an isolation test is required here if we were to add a validity test, because you can cause a failure in the REINDEX with a set of SQLs in a single session. I'm OK to add a test, perhaps with a NOTICE set when the safe flag is true. Anyway, what you have is more than enough to prove your point. Thanks for that. -- Michael
signature.asc
Description: PGP signature
