Thomas Munro <thomas.mu...@enterprisedb.com> writes: > On Tue, Mar 27, 2018 at 11:58 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> I wonder whether we shouldn't mark *all* of these parallel-unsafe. >> I'm not exactly convinced that 'restricted' is sufficient for the >> others, and even if it is, there's certainly little if any upside >> for letting them be executed in parallel-enabled mode.
> Yeah, I almost sent a patch to do exactly that, but then I realised > the others all just set a global variable so they're technically fine > as 'r'. Yeah, so I see after looking closer. > I have no strong preference either way; these functions will > only actually be run in parallel in the weird situation of > force_parallel_mode = on. While the minimal patch would be fine for now, what I'm worried about is preventing future mistakes. It seems highly likely that the reason binary_upgrade_create_empty_extension is marked 'r' is that somebody copied-and-pasted that from another binary_upgrade_foo function without thinking very hard. Marking them all 'u' would help to forestall future mistakes of the same sort, while leaving them as 'r' doesn't seem to buy us anything much (beyond a smaller catalog patch today). Querying for other functions marked 'r' leaves me with some other related doubts: 1. Why are the various flavors of pg_get_viewdef() marked 'r'? Surely reading the catalogs is a thing parallel children are allowed to do. If there is a good reason for pg_get_viewdef() to be 'r', why doesn't the same reason apply to all the other ruleutils functions? 2. Why are the various thingy-to-xml functions marked 'r'? Again it can't be because they read catalogs or data. I can imagine that the reason for restricting cursor_to_xml is that the cursor might execute parallel-unsafe operations, but then why isn't it marked 'u'? 3. Isn't pg_import_system_collations() unsafe, for the same reason as binary_upgrade_create_empty_extension()? regards, tom lane