Horton

Re: BUG #18314: PARALLEL UNSAFE function does not prevent parallel index build

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: jian he <jian.universality@gmail.com>
Cc: Tender Wang <tndrwang@gmail.com>, pgsql-bugs@lists.postgresql.org, Alexander Lakhin <exclusion@gmail.com>
Date: 2024-03-05T22:45:21Z
Lists: pgsql-bugs 
On Tue, Mar 05, 2024 at 09:22:08PM +0800, jian he wrote:
> in RelationGetIndexClause to, I think you can use the following to
> save a SearchSysCache1 cycle?
> if (relation->rd_indextuple == NULL ||
> heap_attisnull(relation->rd_indextuple, Anum_pg_index_indexprs, NULL))
> return NIL;
> 
> or
> if (relation->rd_indextuple == NULL ||
> heap_attisnull(relation->rd_indextuple, Anum_pg_index_indpred, NULL))
> return NIL;

Don't think so.  The point is to not rely on the relcache at all to
retrieve this information.

> main question would be why not two functions,
> like RelationGetIndexRawExpr(Relation relation),
> RelationGetIndexRawPred(Relation relation)

This comes down to if it is clean to have references to the catalog
pg_index in the planner, which is not the case yet so my take is that
two functions is much cleaner even if both return a List.

Anyway, why do you insist in putting the new functions in relcache.c?
I would suggest to move that to lsyscache.c instead, close to
get_index_column_opclass where there are routines for the syscache of
pg_index.  It would be possible to reuse that in the reindex code, for
example.

The patch should add a comment in in plan_create_index_workers()
explaining why we care about raw expressions and indexes rather than
the relcache information.
--
Michael

Commits

  1. Revert "Fix parallel-safety check of expressions and predicate for index builds"

  2. Fix parallel-safety check of expressions and predicate for index builds

  3. Ensure we preprocess expressions before checking their volatility.