Re: Parallel safety tagging of extension functions

On Tue, Jun 14, 2016 at 1:51 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Tue, Jun 14, 2016 at 6:37 AM, Andreas Karlsson <andreas(at)proxel(dot)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.

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

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company