Re: Parallel safety tagging of extension functions

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Andreas Karlsson <andreas(at)proxel(dot)se>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Parallel safety tagging of extension functions
Date: 2016-05-20 23:37:49
Message-ID: CA+TgmobHur8MDUo1FJerBwP+L6LCWQt2F7cHAwsp9sB9AHih1A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, May 19, 2016 at 5:50 PM, Andreas Karlsson <andreas(at)proxel(dot)se> wrote:
> I have gone through all our extensions and tried to tag all functions
> correctly according to their parallel safety.
>
> I also did the same for the aggregate functions in a second patch, and for
> min(citext)/max(citext) set a COMBINEFUNC.
>
> The changes for the functions is attached as one huge patch. Feel free to
> suggest a way to split it up or change it in any way if that would make it
> easier to review/apply.
>
> Some open questions:
>
> - How should we modify the aggregate functions when upgrading extensions?
> ALTER AGGREGATE cannot change COMBINEFUNC or PARALLEL. My current patch
> updates the system catalogs directly, which should be safe in this case, but
> is this an acceptable solution?
>
> - Do you think we should add PARALLEL UNSAFE to the functions which we know
> are unsafe to make it obvious that it is intentional?
>
> - I have not added the parallel tags to the functions used by our procedural
> languages. Should we do so?
>
> - I have marked uuid-ossp, chkpass_in() and pgcrypto functions which
> generate random data as safe, despite that they depend on state in the
> backend. My reasoning is that, especially for the pgcrypto functions, that
> nobody should not rely on the PRNG state. For uuid-ossp I am on the fence.
>
> - I have touched a lot of legacy libraries, like tsearch2 and the spi/*
> stuff. Is that a good idea?
>
> - I decided to ignore that isn_weak() exists (and would make all input
> functions PARALLEL RESTRICTED) since it is only there is ISN_WEAK_MODE is
> defined. Is that ok?

I guess my first question is whether we have consensus on the release
into which we should put this. Some people (Noah, among others)
thought it should wait because we're after feature freeze, while
others thought we should do it now. If we're going to try to get this
into 9.6, I'll work on reviewing this sooner rather than later, but if
we're not going to do that I'm going to postpone dealing with it until
after we branch.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-05-20 23:40:53 Re: Lets (not) break all the things. Was: [pgsql-advocacy] 9.6 -> 10.0
Previous Message Robert Haas 2016-05-20 23:34:45 Re: [PATCH] Add EXPLAIN (ALL) shorthand