Horton

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

jian he <jian.universality@gmail.com>

View thread View on postgr.es
From: jian he <jian.universality@gmail.com>
To: Tender Wang <tndrwang@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, pgsql-bugs@lists.postgresql.org, Alexander Lakhin <exclusion@gmail.com>
Date: 2024-03-05T13:22:08Z
Lists: pgsql-bugs 
On Tue, Mar 5, 2024 at 7:49 PM Tender Wang <tndrwang@gmail.com> wrote:
>
>>
>> On top of that, your approach has two bugs:
>> RelationGetIndexExpressions and RelationGetIndexPredicate would return
>> the flattened information if available directly from the relcache, so
>> even if you pass get_raw_expr=true you may finish with flattened
>> expressions and/or predicates, facing the same issues.
>>
>> I think that the correct way to do that would be to get the
>> information from the syscache when calculating the number of parallel
>> workers to use for the parallel index builds, with SysCacheGetAttr(),
>> for example.  That would ensure that the expressions are not
>> flattened, letting the relcache be.
>
>
> I refactor the v2 version patch according to your advice.
> Any thoughts?

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;

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

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.