> > 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! > > > > I think it is difficult to say for what purpose parallel-unsafe function got > called in > parallel context so if we give an error in cases where otherwise it could > lead to > a crash or caused other horrible things, users will probably appreciate us. > OTOH, if the parallel-safety labeling is wrong (parallel-safe function is > marked > parallel-unsafe) and we gave an error in such a case, the user can always > change > the parallel-safety attribute by using Alter Function. > Now, if adding such a check is costly or needs some major re-design then > probably it might not be worth whereas I don't think that is the case for > non-built-in function invocation.
Temporarily, Just in case someone want to take a look at the patch for the safety check. I splited the patch into 0001(parallel safety check for user define function), 0003(parallel safety check for builtin function) and the fix for testcases. IMO, With such a check to give an error when detecting parallel unsafe function in parallel mode, it will be easier for users to discover potential threats(parallel unsafe function) in parallel mode. I think users is likely to invoke parallel unsafe function inner a parallel safe function unintentionally. Such a check can help they detect the problem easier. Although, the strict check limits some usages(intentionally wrapper function) like Robert-san said. To mitigate the effect of the limit, I was thinking can we do the safety check conditionally, such as only check the top function invoke and/or introduce a guc option to control whether do the strict parallel safety check? Thoughts ? Best regards, houzj
0003-check-built-in-function-parallel-safety-in-fmgr_info.patch
Description: 0003-check-built-in-function-parallel-safety-in-fmgr_info.patch
0004-fix-builtin-parallel-safety-label-in-testcase.patch
Description: 0004-fix-builtin-parallel-safety-label-in-testcase.patch
0001-check-UDF-parallel-safety-in-fmgr_info.patch
Description: 0001-check-UDF-parallel-safety-in-fmgr_info.patch
0002-fix-UDF-parallel-safety-label-in-testcase.patch
Description: 0002-fix-UDF-parallel-safety-label-in-testcase.patch
