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

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