On Mon, Jun 7, 2021 at 11:33 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > Note the error is raised after applying the patch, without the patch, > the above won't show any error (error message could be improved here). > Such cases can lead to unpredictable behavior without a patch because > we won't be able to detect the execution of parallel-unsafe functions. > There are similar examples from regression tests. Now, one way to deal > with similar cases could be that we document them and say we don't > consider parallel-safety in such cases and the other way is to detect > such cases and error out. Yet another way could be that we somehow try > to check these cases as well before enabling parallelism but I thought > these cases fall in the similar category as aggregate's support > functions.
I'm not very excited about the idea of checking type input and type output functions. It's hard to imagine someone wanting to do something parallel-unsafe in such a function, unless they're just trying to prove a point. So I don't think checking it would be a good investment of CPU cycles. If we do anything at all, I'd vote for just documenting that such functions should be parallel-safe and that their parallel-safety marks are not checked when they are used as type input/output functions. Perhaps we ought to document the same thing with regard to opclass support functions, another place where it's hard to imagine a realistic use case for doing something parallel-unsafe. In the case of aggregates, I see the issues slightly differently. I don't know that it's super-likely that someone would want to create a parallel-unsafe aggregate function, but I think there should be a way to do it, just in case. However, if somebody wants that, they can just mark the aggregate itself unsafe. There's no benefit for the user to marking the aggregate safe and the support functions unsafe and hoping that the system figures it out somehow. In my opinion, you're basically taking too pure a view of this. We're not trying to create a system that does such a good job checking parallel safety markings that nobody can possibly find a thing that isn't checked no matter how hard they poke around the dark corners of the system. Or at least we shouldn't be trying to do that. We should be trying to create a system that works well in practice, and gives people the flexibility to easily avoid parallelism when they have a query that is parallel-unsafe, while still getting the benefit of parallelism the rest of the time. I don't know what all the cases you've uncovered are, and maybe there's something in there that I'd be more excited about changing if I knew what it was, but the particular problems you're mentioning here seem more theoretical than real to me. -- Robert Haas EDB: http://www.enterprisedb.com
