Re: Parallel safety of binary_upgrade_create_empty_extension

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Mar 26, 2018 at 7:23 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> 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).

> I'm not sure that deliberately mismarking things in the hopes that it
> will make blind cut-and-paste a sensible approach is a very good
> strategy.

So, your proposal is to do nothing and just hope we don't make the same
mistake again in future?

>> 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?

> The only problem with running those in a worker (that I know about) is
> that any locks they acquire wouldn't be retained. But that is
> technically a difference in semantics.

Meh. It's not documented that pg_get_viewdef takes any locks, and
I'm sure most users would be astonished to learn that, and this
argument surely doesn't explain why pg_get_viewdef is restricted
while pg_get_ruledef is marked safe.

> Regarding cursor_to_xml, I don't think the problem you mention exists,
> because the query the cursor runs is independently subject to the
> parallel restrictions.

Well, "independently" is exactly the point. If the cursor query contains
some parallel-unsafe (not just parallel-restricted) operations, and we
allow cursor_to_xml to be parallel-restricted, don't we end up risking
executing parallel-unsafe operations while doing parallelism?

Or to be concrete:

regression=# create sequence s1;
CREATE SEQUENCE
regression=# begin;
BEGIN
regression=# set force_parallel_mode to 1;
SET
regression=# declare c cursor for select nextval('s1');
DECLARE CURSOR
regression=# select cursor_to_xml('c',1,true,true,'');
ERROR: cannot execute nextval() during a parallel operation

I think this behavior is a bug.

regards, tom lane