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

On Thu, May 6, 2021 at 3:00 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> 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. I think that is a good safety net especially if we can do
> it with some simple check. Now, we already have pg_proc information in
> fmgr_info_cxt_security for non-built-in functions, so we can check
> that and error out if the unsafe function is invoked in parallel mode.
> It has been observed that we were calling some unsafe functions in
> parallel-mode in the regression tests which is caught by such a check.

I see your point, but I am not convinced. As I said to Tsunakawa-san,
doing the check here seems expensive. Also, I had the idea in mind
that parallel-safety should work like volatility. We don't check at
runtime whether a volatile function is being called in a context where
volatile functions are not supposed to be used. If for example you try
to call a volatile function directly from an index expression I
believe you will get an error. But if the index expression calls an
immutable function and then that function internally calls something
volatile, you don't get an error. Now it might not be a good idea: you
could end up with a broken index. But that's your fault for
mislabeling the function you used.

Sometimes this is actually quite useful. You might know that, while
the function is in general volatile, it is immutable in the particular
way that you are using it. Or, perhaps, you are using the volatile
function incidentally and it doesn't affect the output of your
function at all. Or, maybe you actually want to build an index that
might break, and then it's up to you to rebuild the index if and when
that is required. Users do this kind of thing all the time, I think,
and would be unhappy if we started checking it more rigorously than we
do today.

Now, I don't see why the same idea can't or shouldn't apply to
parallel-safety. If you call a parallel-unsafe function in a parallel
context, it's pretty likely that you are going to get an error, and so
you might not want to do it. If the function is written in C, it could
even cause horrible things to happen so that you crash the whole
backend or something, but I tried to set things up so that for
built-in functions you'll just get an error. But on the other hand,
maybe the parallel-unsafe function you are calling is not
parallel-unsafe in all cases. If you want to create a wrapper function
that is labelled parallel-safe and try to make that it only calls the
parallel-unsafe function in the cases where there's no safety problem,
that's up to you!

It's possible that I had the wrong idea here, so maybe the question
deserves more thought, but I wanted to explain what my thought process
was.

--
Robert Haas
EDB: http://www.enterprisedb.com