Re: Parallel INSERT SELECT take 2

On Wed, Apr 28, 2021 at 12:44 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> 0003:
> 1) Temporarily, add the check of built-in function by adding a member for proparallel in FmgrBuiltin.
> To avoid enlarging FmgrBuiltin struct , change the existing bool members strict and and retset into
> one member of type char, and represent the original values with some bit flags.
>

I was thinking that it would be better to replace the two bool members
with one "unsigned char" member for the bitflags for strict and
retset, and another "char" member for parallel.
The struct would still remain the same size as it originally was, and
you wouldn't need to convert between bit settings and char
('u'/'r'/'s') each time a built-in function was checked for
parallel-safety in fmgr_info().

> Note: this will lock down the parallel property of built-in function, but, I think the parallel safety of built-in function
> is related to the C code, user should not change the property of it unless they change its code. So, I think it might be
> better to disallow changing parallel safety for built-in functions, Thoughts ?
>

I'd vote for disallowing it (unless someone can justify why it
currently is allowed).

> I have not added the parallel safety check in ALTER/CREATE table PARALLEL DML SAFE command.
> I think it seems better to add it after some more discussion.
>

I'd vote for not adding such a check (as this is a declaration).

Some additional comments:

1) In patch 0002 comment, it says:
This property is recorded in pg_class's relparallel column as 'u', 'r', or 's',
just like pg_proc's proparallel. The default is UNSAFE.
It should say "relparalleldml" column.

2) With the patches applied, I seem to be getting a couple of errors
when running "make installcheck-world" with
force_parallel_mode=regress in effect.
Can you please try it?

Regards,
Greg Nancarrow
Fujitsu Australia