Re: Unexpected result from ALTER FUNCTION— looks like a bug

Hi,

On Tue, Apr 19, 2022 at 07:21:19PM -0700, David G. Johnston wrote:
> On Tue, Apr 19, 2022 at 7:07 PM Bryn Llewellyn <bryn(at)yugabyte(dot)com> wrote:
>
> > *SUMMARY*
> >
> > This part of the syntax diagram for "alter function":
> >
> > *ALTER FUNCTION name [ ( [ [ argmode ] [ argname ] argtype [, ...] ] ) ]
> > action [ … ]*
> >
> > says that the first "action" can be followed (without punctuation) by
> > zero, one, or many other actions. A semantic rule says that no particular
> > action can be specified more than once. My tests used these possible
> > actions:
> >
> > *alter function s1.f()security invokerset timezone = 'UTC'stable*
> > *parallel safe*
> > *;*
> >
> > It brings this new status:
> >
> >
> >
> > * name | type | security | proconfig
> > | volatility
> > | parallel ------+------+----------+---------------------------------------------------------+------------+------------ f
> > | func | invoker | {TimeZone=UTC}
> > | stable | restricted*
> >
> > This is the bug.
> >
>
> It has room for improvement from a user experience perspective.
>
> While I haven't experimented with this for confirmation, what you are
> proposing here (set + parallel safe) is an impossible runtime combination
> (semantic rule) but perfectly valid to write syntactically. Your function
> must either be restricted or unsafe per the rules for specifying parallel
> mode.
>
> If this is indeed what is happening then the documentation should make note
> of it. Whether the server should emit a notice or warning in this
> situation is less clear. I'm doubting we would introduce an error at this
> point but probably should have when parallelism was first added to the
> system.

That's not the problem here though, as you can still end up with the wanted
result using 2 queries. Also, the PARALLEL part is entirely ignored, so if you
wanted to mark the function as PARALLEL UNSAFE because you're also doing a SET
that would make it incompatible it would also be ignored, same if you use a
RESET clause.

AFAICT the problem is that SET / RESET part is messing with the HeapTuple, so
you can't use the procForm reference afterwards. Simply processing
parallel_item before set_items fixes the problem, as in the attached.