On Wed, Apr 28, 2021 at 9:42 PM houzj.f...@fujitsu.com <houzj.f...@fujitsu.com> wrote: > So, If we do not want to lock down the parallel safety of built-in functions. > It seems we can try to fetch the proparallel from pg_proc for built-in > function > in fmgr_info_cxt_security too. To avoid recursive safety check when fetching > proparallel from pg_proc, we can add a Global variable to mark is it in a > recursive state. > And we skip safety check in a recursive state, In this approach, parallel > safety > will not be locked, and there are no new members in FmgrBuiltin. > > Attaching the patch about this approach [0001-approach-1]. > Thoughts ?
This seems to be full of complicated if-tests that don't seem necessary and aren't explained by the comments. Also, introducing a system cache lookup here seems completely unacceptable from a reliability point of view, and I bet it's not too good for performance, either. > I also attached another approach patch [0001-approach-2] about adding > parallel safety in FmgrBuiltin, because this approach seems faster and > we can combine some bool member into a bitflag to avoid enlarging the > FmgrBuiltin array, though this approach will lock down the parallel safety > of built-in function. This doesn't seem like a good idea either. I really don't understand what problem any of this is intended to solve. Bharath's analysis above seems right on point to me. I think if anybody is writing a patch that requires that this be changed in this way, that person is probably doing something wrong. -- Robert Haas EDB: http://www.enterprisedb.com
