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

From: Robert Haas <robertmhaas@gmail.com>
> On Wed, Apr 28, 2021 at 9:42 PM houzj.fnst@fujitsu.com
> <houzj.fnst@fujitsu.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
safetyof 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().
Regardingthe INSERT part, we're trying to rely on the parallel safety of the target table that the user specified with
CREATE/ALTERTABLE.  I don't see where we need to check the parallel safety of uilt-in functions.
 


Regards
Takayuki Tsunakawa