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

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Tender Wang <tndrwang(at)gmail(dot)com>
Cc: Alexander Lakhin <exclusion(at)gmail(dot)com>, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #18314: PARALLEL UNSAFE function does not prevent parallel index build
Date: 2024-02-08 06:17:53
Message-ID: CACJufxEGcGYg62qMzfkkx_0v5QWvM-vMwz9166hDpV8Emi0OZw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Wed, Feb 7, 2024 at 11:43 PM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> On Wed, Feb 7, 2024 at 7:54 PM Tender Wang <tndrwang(at)gmail(dot)com> wrote:
> >
> > In pg document, I found this:
> >
> > Also, a block containing an EXCEPTION clause effectively forms a subtransaction that can be rolled back without affecting the outer transaction.
> >
> > So it will report error for this case even though we replace PARALLEL UNSAFE with PARALLEL SAFE.
> >
> > I think if PARALLEL UNSAFE is specified by users, PG internal should not choose to build index parallel, even if the function is
> > too simply that can be transform to Const node.
> >
> > Attached patch will fix Alexander reported issue. I choose pass the raw index expression to is_parallel_safe().
> > The comments of RelationGetIndexExpressions() doesn't say it will return optimized expression. So I add a bool argument to RelationGetIndexExpressions().
> > I don't decide to write a new function like RelationGetIndexRawExpression. I think the RelationGetIndexExpressions() is enough after adding a bool argument to indicate
> > whether caller needing a raw index expression.
> >
> > But if users specify PARALLEL SAFE, like I said before, it also reports error. But I think it is another thing. Maybe it is reasonable?
> >

based on index_build's ereport(DEBUG1...) output information
you patch disabled parallel index build for parallel safe function.

after apply your patch:
set log_min_messages to debug1;
begin;
drop table if exists btree_para_bld;
CREATE TABLE btree_para_bld(i int);
ALTER TABLE btree_para_bld SET (parallel_workers = 4);
SET local max_parallel_maintenance_workers TO 4;
CREATE or replace FUNCTION para_unsafe_f1() RETURNS int IMMUTABLE
PARALLEL UNSAFE
AS $$
BEGIN
RETURN 0;
END$$ LANGUAGE plpgsql;

CREATE or replace FUNCTION para_safe() RETURNS int IMMUTABLE PARALLEL UNSAFE
AS $$
BEGIN
RETURN 0;
END$$ LANGUAGE plpgsql;
CREATE INDEX ON btree_para_bld((para_unsafe_f1()));
CREATE INDEX ON btree_para_bld((para_safe()));
commit;

debug information:
2024-02-08 14:11:26.664 CST [33245] DEBUG: building index
"btree_para_bld_para_unsafe_f1_idx" on table "btree_para_bld" serially
2024-02-08 14:11:26.664 CST [33245] DEBUG: index
"btree_para_bld_para_unsafe_f1_idx" can safely use deduplication
CREATE INDEX
2024-02-08 14:11:26.666 CST [33245] DEBUG: building index
"btree_para_bld_para_safe_idx" on table "btree_para_bld" serially
2024-02-08 14:11:26.666 CST [33245] DEBUG: index
"btree_para_bld_para_safe_idx" can safely use deduplication
CREATE INDEX

the problem entry point is_parallel_safe, i think.

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Etsuro Fujita 2024-02-08 12:06:38 Re: BUG #17828: postgres_fdw leaks file descriptors on error and aborts aborted transaction in lack of fds
Previous Message Masahiko Sawada 2024-02-08 05:54:52 Re: Potential data loss due to race condition during logical replication slot creation