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

Thanks for pushing the patch.

I found a typos:
/*
+ * get_index_expressions
+ *
+ *             Given the index OID, its a List of its expressions or NIL
if none.
+ */
"its" should be "return"?


Tender Wang <tndrwang@gmail.com> 于2024年3月6日周三 12:30写道：

>
>
> Michael Paquier <michael@paquier.xyz> 于2024年3月6日周三 06:45写道：
>
>> 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.
>>
>
> Make sense.
> Please review attached v4 patch.
>
>>
>> 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.
>>
>
> Added
>
> --
>> Michael
>>
>
>
> --
> Tender Wang
> OpenPie:  https://en.openpie.com/
>


-- 
Tender Wang
OpenPie:  https://en.openpie.com/