RE: [bug?] Missed parallel safety checks, and wrong parallel safety

From: Robert Haas <robertmhaas(at)gmail(dot)com>
> On Wed, Apr 28, 2021 at 9:42 PM houzj(dot)fnst(at)fujitsu(dot)com
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > So, If we do not want to lock down the parallel safety of built-in functions.
> > It seems we can try to fetch the proparallel from pg_proc for built-in function
> > in fmgr_info_cxt_security too. To avoid recursive safety check when fetching
> > proparallel from pg_proc, we can add a Global variable to mark is it in a
> recursive state.
> > And we skip safety check in a recursive state, In this approach, parallel safety
> > will not be locked, and there are no new members in FmgrBuiltin.
> >
> > Attaching the patch about this approach [0001-approach-1].
> > Thoughts ?
>
> This seems to be full of complicated if-tests that don't seem
> necessary and aren't explained by the comments. Also, introducing a
> system cache lookup here seems completely unacceptable from a
> reliability point of view, and I bet it's not too good for
> performance, either.

Agreed. Also, PG_TRY() would be relatively heavyweight here. I'm inclined to avoid this approach.

> > I also attached another approach patch [0001-approach-2] about adding
> > parallel safety in FmgrBuiltin, because this approach seems faster and
> > we can combine some bool member into a bitflag to avoid enlarging the
> > FmgrBuiltin array, though this approach will lock down the parallel safety
> > of built-in function.
>
> This doesn't seem like a good idea either.

This looks good to me. What makes you think so?

That said, I actually think we want to avoid even this change. That is, I'm wondering if we can skip the parallel safety of built-in functions.

Can anyone think of the need to check the parallel safety of built-in functions in the context of parallel INSERT SELECT? The planner already checks (or can check) the parallel safety of the SELECT part with max_parallel_hazard(). Regarding the INSERT part, we're trying to rely on the parallel safety of the target table that the user specified with CREATE/ALTER TABLE. I don't see where we need to check the parallel safety of uilt-in functions.

Regards
Takayuki Tsunakawa