Re: Parallel safety of binary_upgrade_create_empty_extension

Thomas Munro <thomas.munro@enterprisedb.com>

From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2018-03-27T02:30:21Z
Lists: pgsql-hackers

Attachments

On Tue, Mar 27, 2018 at 12:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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()?

Yeah.  I hacked something up in Python to analyse the C call graph and
look for non-PARALLEL SAFE functions written in C that can reach
AssignTransactionId.  Attached, for interest.  Not a great approach
because current_schema, fetch_search_path, SPI_XXX and a couple of
others all lead there creating many possibly false positives (though
who knows).  If I filter those out I'm left with the ones already
mentioned (pg_import_system_collations,
binary_upgrade_create_empty_extension) plus two others:

1.  unique_key_recheck, not user callable anyway.
2.  brin_summarize_range is marked 's'.  Seems wrong.

-- 
Thomas Munro
http://www.enterprisedb.com

Commits

  1. Fix bogus provolatile/proparallel markings on a few built-in functions.