Re: Parallel safety tagging of extension functions

On 06/14/2016 09:55 PM, Robert Haas wrote:
> On Tue, Jun 14, 2016 at 1:51 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Tue, Jun 14, 2016 at 6:37 AM, Andreas Karlsson <andreas@proxel.se> wrote:
>>> I have rebased all my patches on the current master now (and skipped the
>>> extensions I previously listed).
>>
>> Thanks, this is really helpful.  It was starting to get hard to keep
>> track of what hadn't been applied yet.  I decided to prioritize
>> getting committed the patches where the extension version had already
>> been bumped by 749a787c5b25ae33b3d4da0ef12aa05214aa73c7, so I've now
>> committed the patches for cube, hstore, intarray, ltree, pg_trgm, and
>> seg.
>
> I've now also committed the patches for sslinfo, unaccept, uuid-ossp, and xml2.

Thanks!

> I took at look at the patch for tsearch2, but I think token_type() is
> mismarked.  You have it marked PARALLEL SAFE but seems to depend on
> the result of GetCurrentParser(), which returns a backend-private
> state variable.

Hm, as far as I can tell that is only for token_type() which I made 
RESTRICTED while token_type(int4) and token_type(text) do not call 
GetCurrentParser().

> That was the only clear mistake I found, but I tend
> to think that changing the markings on anything defined by
> UNSUPPORTED_FUNCTION() is pretty silly, because there's no point in
> going to extra planner effort to generate a parallel plan only to
> error out as soon as we try to execute it.  I think you should leave
> all of those out of the patch.

I will fix this.

> I also took a look at the patch for tablefunc.  I think that you've
> got the markings right, here, but I think that it would be good to add
> PARALLEL UNSAFE explicitly to the 1.1 version of the file for the
> functions are unsafe, and add a comment like "-- query might do
> anything" or some other indication as to why they are so marked, for
> the benefit of future readers.

Good suggestion.

Andreas