RE: [bug?] Missed parallel safety checks, and wrong parallel safety - Mailing list pgsql-hackers

From tsunakawa.takay@fujitsu.com
Subject RE: [bug?] Missed parallel safety checks, and wrong parallel safety
Date
Msg-id TYAPR01MB2990A71A7D2A7CAF0A812507FE469@TYAPR01MB2990.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: [bug?] Missed parallel safety checks, and wrong parallel safety  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses RE: [bug?] Missed parallel safety checks, and wrong parallel safety  ("houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>)
List pgsql-hackers
From: Tom Lane <tgl@sss.pgh.pa.us>
> Amit Kapila <amit.kapila16@gmail.com> writes:
> > IIUC, the idea here is to check for parallel safety of functions at
> > someplace in the code during function invocation so that if we execute
> > any parallel unsafe/restricted function via parallel worker then we
> > error out. If so, isn't it possible to deal with built-in and
> > non-built-in functions in the same way?
>
> Yeah, one of the reasons I doubt this is a great idea is that you'd
> still have to fetch the pg_proc row for non-built-in functions.
>
> The obvious place to install such a check is fmgr_info(), which is
> fetching said row anyway for other purposes, so it's really hard to
> see how adding anything to FmgrBuiltin is going to help.

Thank you, fmgr_info() looks like the best place to do the parallel safety check.  Having a quick look at its callers,
Ididn't find any concerning place (of course, we can't be relieved until the regression test succeeds.)  Also, with
fmgr_info(),we don't have to find other places to add the check to deal with functions calls in execExpr.c and
execExprInterp.c. This is beautiful. 

But the current fmgr_info() does not check the parallel safety of builtin functions.  It does not have information to
dothat.  There are two options.  Which do you think is better?  I think 2. 

1) fmgr_info() reads pg_proc like for non-builtin functions
This ruins the effort for the fast path for builtin functions.  I can't imagine how large the adverse impact on
performancewould be, but I'm worried. 

The benefit is that ALTER FUNCTION on builtin functions takes effect.  But such operations are nonsensical, so I don't
thinkwe want to gain such a benefit. 


2) Gen_fmgrtab.pl adds a member for proparallel in FmgrBuiltin
But we don't want to enlarge FmgrBuiltin struct.  So, change the existing bool members strict and and retset into one
memberof type char, and represent the original values with some bit flags.  Then we use that member for proparallel as
well. (As a result, one byte is left for future use.) 


I think we'll try 2).  I'd be grateful if you could point out anything I need to be careful to.


Regards
Takayuki Tsunakawa





pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: TRUNCATE on foreign table
Next
From: Michael Paquier
Date:
Subject: Re: multi-install PostgresNode fails with older postgres versions