Thread: IMMUTABLE and PARALLEL SAFE function markings
I have defined a function "is_not_distinct" for the purpose of creating a custom operator "===".
CREATE FUNCTION is_not_distinct(a anyelement, b anyelement)
returns boolean
language sql as $$
select a is not distinct from b;
$$
IMMUTABLE;
CREATE OPERATOR === (
LEFTARG = anyelement,
RIGHTARG = anyelement,
PROCEDURE = is_not_distinct,
NEGATOR = !==
);
I have observed that the resulting queries were executed without using parallelisation.
I have learned by asking on Freenode that the reason my queries are not using parallelisation is because I have not configured PARALLEL SAFE.
I find it counter-intuitive that a function with IMMUTABLE marking would require an explicit PARALLEL SAFE. It would seem logical that in all cases where a function is appropriately market as IMMUTABLE it would also be PARALLEL SAFE.
I was wondering what is the reason IMMUTABLE functions are not by default PARALLEL SAFE and if the default behaviour could be changed to make IMMUTABLE functions PARALLEL SAFE?
Thank you,
Gajus
On 26/11/2018 22:23, Gajus Kuizinas wrote: > I was wondering what is the reason IMMUTABLE functions are not by > default PARALLEL SAFE and if the default behaviour could be changed to > make IMMUTABLE functions PARALLEL SAFE? I think I have to concur with this. When is an immutable function not parallel safe? Sure it could be mislabeled as immutable but it could just as easily be mislabeled as parallel safe. And we already treat fake immutable functions as user errors, for example in indexes. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Hi,
On 2018-11-27 00:33:10 +0100, Vik Fearing wrote:
> On 26/11/2018 22:23, Gajus Kuizinas wrote:
> > I was wondering what is the reason IMMUTABLE functions are not by
> > default PARALLEL SAFE and if the default behaviour could be changed to
> > make IMMUTABLE functions PARALLEL SAFE?
>
> I think I have to concur with this. When is an immutable function not
> parallel safe?
>
> Sure it could be mislabeled as immutable but it could just as easily be
> mislabeled as parallel safe. And we already treat fake immutable
> functions as user errors, for example in indexes.
I think it'd introduce more problems than it'd solve. Either you ignore
the proparallel setting - resulting in broken catalog querying - or you
have to have a decent amount of special behaviour that an explicit ALTER
FUNCTION ... IMMUTABLE | STABLE | VOLATILE and SET PARALLEL { UNSAFE
| RESTRICTED | SAFE } would also need to change the respective other
category.
Greetings,
Andres Freund
On 27/11/2018 00:39, Andres Freund wrote:
> Hi,
>
> On 2018-11-27 00:33:10 +0100, Vik Fearing wrote:
>> On 26/11/2018 22:23, Gajus Kuizinas wrote:
>>> I was wondering what is the reason IMMUTABLE functions are not by
>>> default PARALLEL SAFE and if the default behaviour could be changed to
>>> make IMMUTABLE functions PARALLEL SAFE?
>>
>> I think I have to concur with this. When is an immutable function not
>> parallel safe?
>>
>> Sure it could be mislabeled as immutable but it could just as easily be
>> mislabeled as parallel safe. And we already treat fake immutable
>> functions as user errors, for example in indexes.
>
> I think it'd introduce more problems than it'd solve. Either you ignore
> the proparallel setting - resulting in broken catalog querying - or you
> have to have a decent amount of special behaviour that an explicit ALTER
> FUNCTION ... IMMUTABLE | STABLE | VOLATILE and SET PARALLEL { UNSAFE
> | RESTRICTED | SAFE } would also need to change the respective other
> category.
Surely a simple rule could be made that provolatile='i' trumps
proparallel. No need to make them agree.
The default catalogs should agree (and I would expect the sanity checks
to look for that) but here we're talking about user functions.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On 2018-11-27 00:47:47 +0100, Vik Fearing wrote:
> On 27/11/2018 00:39, Andres Freund wrote:
> > Hi,
> >
> > On 2018-11-27 00:33:10 +0100, Vik Fearing wrote:
> >> On 26/11/2018 22:23, Gajus Kuizinas wrote:
> >>> I was wondering what is the reason IMMUTABLE functions are not by
> >>> default PARALLEL SAFE and if the default behaviour could be changed to
> >>> make IMMUTABLE functions PARALLEL SAFE?
> >>
> >> I think I have to concur with this. When is an immutable function not
> >> parallel safe?
> >>
> >> Sure it could be mislabeled as immutable but it could just as easily be
> >> mislabeled as parallel safe. And we already treat fake immutable
> >> functions as user errors, for example in indexes.
> >
> > I think it'd introduce more problems than it'd solve. Either you ignore
> > the proparallel setting - resulting in broken catalog querying - or you
> > have to have a decent amount of special behaviour that an explicit ALTER
> > FUNCTION ... IMMUTABLE | STABLE | VOLATILE and SET PARALLEL { UNSAFE
> > | RESTRICTED | SAFE } would also need to change the respective other
> > category.
>
> Surely a simple rule could be made that provolatile='i' trumps
> proparallel. No need to make them agree.
>
> The default catalogs should agree (and I would expect the sanity checks
> to look for that) but here we're talking about user functions.
I think it'd be entirely unacceptable that
SELECT * FROM pg_proc WHERE proparallel = 's'
wouldn't actually return all the functions that are parallel safe.
Greetings,
Andres Freund
Greetings,
* Andres Freund (andres@anarazel.de) wrote:
> On 2018-11-27 00:47:47 +0100, Vik Fearing wrote:
> > On 27/11/2018 00:39, Andres Freund wrote:
> > > On 2018-11-27 00:33:10 +0100, Vik Fearing wrote:
> > >> On 26/11/2018 22:23, Gajus Kuizinas wrote:
> > >>> I was wondering what is the reason IMMUTABLE functions are not by
> > >>> default PARALLEL SAFE and if the default behaviour could be changed to
> > >>> make IMMUTABLE functions PARALLEL SAFE?
> > >>
> > >> I think I have to concur with this. When is an immutable function not
> > >> parallel safe?
> > >>
> > >> Sure it could be mislabeled as immutable but it could just as easily be
> > >> mislabeled as parallel safe. And we already treat fake immutable
> > >> functions as user errors, for example in indexes.
> > >
> > > I think it'd introduce more problems than it'd solve. Either you ignore
> > > the proparallel setting - resulting in broken catalog querying - or you
> > > have to have a decent amount of special behaviour that an explicit ALTER
> > > FUNCTION ... IMMUTABLE | STABLE | VOLATILE and SET PARALLEL { UNSAFE
> > > | RESTRICTED | SAFE } would also need to change the respective other
> > > category.
> >
> > Surely a simple rule could be made that provolatile='i' trumps
> > proparallel. No need to make them agree.
> >
> > The default catalogs should agree (and I would expect the sanity checks
> > to look for that) but here we're talking about user functions.
>
> I think it'd be entirely unacceptable that
> SELECT * FROM pg_proc WHERE proparallel = 's'
> wouldn't actually return all the functions that are parallel safe.
Agreed, but I could see us having a regression test which complains if
it finds any which are marked as immutable but aren't parallel safe.
That said, I do *not* think we should make any assumptions here- users
incorrectly mark things all the time but we shouldn't encourage that and
we shouldn't assume that functions marked as immutable are parallel
safe.
Thanks!
Stephen
Attachment
On 27/11/2018 01:05, Stephen Frost wrote: > That said, I do *not* think we should make any assumptions here- users > incorrectly mark things all the time but we shouldn't encourage that and > we shouldn't assume that functions marked as immutable are parallel > safe. Does that mean we also shouldn't assume that functions marked as immutable are index safe? User error is user error. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachment
Hi, On 2018-11-26 19:05:02 -0500, Stephen Frost wrote: > Agreed, but I could see us having a regression test which complains if > it finds any which are marked as immutable but aren't parallel safe. That doesn't help if a user writes a query to review the not parallel safe functions in their installation. Greetings, Andres Freund
Greetings, * Vik Fearing (vik.fearing@2ndquadrant.com) wrote: > On 27/11/2018 01:05, Stephen Frost wrote: > > That said, I do *not* think we should make any assumptions here- users > > incorrectly mark things all the time but we shouldn't encourage that and > > we shouldn't assume that functions marked as immutable are parallel > > safe. > > Does that mean we also shouldn't assume that functions marked as > immutable are index safe? We've got an index safe flag? That's news to me. Thanks! Stephen
Attachment
On 27/11/2018 01:10, Stephen Frost wrote: > Greetings, > > * Vik Fearing (vik.fearing@2ndquadrant.com) wrote: >> On 27/11/2018 01:05, Stephen Frost wrote: >>> That said, I do *not* think we should make any assumptions here- users >>> incorrectly mark things all the time but we shouldn't encourage that and >>> we shouldn't assume that functions marked as immutable are parallel >>> safe. >> >> Does that mean we also shouldn't assume that functions marked as >> immutable are index safe? > > We've got an index safe flag? Yes. It's called provolatile='i'. > That's news to me. > > Thanks! You're welcome. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachment
Greetings, * Andres Freund (andres@anarazel.de) wrote: > On 2018-11-26 19:05:02 -0500, Stephen Frost wrote: > > Agreed, but I could see us having a regression test which complains if > > it finds any which are marked as immutable but aren't parallel safe. > > That doesn't help if a user writes a query to review the not parallel > safe functions in their installation. I'm really not sure what you're getting at here..? Parallel safe functions should be marked as such. Immutable functions should be marked as such. We should not assume that one implies the other, nor should we operate as if they do. My suggestion for a regression test was to make PG developers really think about if their new immutable functions should also be marked as parallel safe, in the event that they forget to mark it as such. If it's really immutable and not parallel safe, then they need to adjust the expected regression test output (and we can all see it...). Thanks! Stephen
Attachment
Greetings, * Vik Fearing (vik.fearing@2ndquadrant.com) wrote: > On 27/11/2018 01:10, Stephen Frost wrote: > > * Vik Fearing (vik.fearing@2ndquadrant.com) wrote: > >> On 27/11/2018 01:05, Stephen Frost wrote: > >>> That said, I do *not* think we should make any assumptions here- users > >>> incorrectly mark things all the time but we shouldn't encourage that and > >>> we shouldn't assume that functions marked as immutable are parallel > >>> safe. > >> > >> Does that mean we also shouldn't assume that functions marked as > >> immutable are index safe? > > > > We've got an index safe flag? > > Yes. It's called provolatile='i'. ... and we complain if someone tries to use a provolatile <> 'i' function directly in an index, so not sure what you're getting at here? Thanks! Stephen
Attachment
On 2018-11-26 19:13:17 -0500, Stephen Frost wrote: > Greetings, > > * Andres Freund (andres@anarazel.de) wrote: > > On 2018-11-26 19:05:02 -0500, Stephen Frost wrote: > > > Agreed, but I could see us having a regression test which complains if > > > it finds any which are marked as immutable but aren't parallel safe. > > > > That doesn't help if a user writes a query to review the not parallel > > safe functions in their installation. > > I'm really not sure what you're getting at here..? You replied to me saying it'd be a bad idea to infer parallel safety from immutability: > > > Surely a simple rule could be made that provolatile='i' trumps > > > proparallel. No need to make them agree. > > > [...] > > > > I think it'd be entirely unacceptable that > > SELECT * FROM pg_proc WHERE proparallel = 's' > > wouldn't actually return all the functions that are parallel safe. > > Agreed, but I could see us having a regression test which complains if > it finds any which are marked as immutable but aren't parallel safe. I'm saying that that doesn't address my concern. > Parallel safe functions should be marked as such. Immutable functions > should be marked as such. We should not assume that one implies the > other, nor should we operate as if they do. > > My suggestion for a regression test was to make PG developers really > think about if their new immutable functions should also be marked as > parallel safe, in the event that they forget to mark it as such. If > it's really immutable and not parallel safe, then they need to adjust > the expected regression test output (and we can all see it...). See http://archives.postgresql.org/message-id/20181126234521.rh3grz7aavx2ubjv%40alap3.anarazel.de Greetings, Andres Freund
Greetings, * Andres Freund (andres@anarazel.de) wrote: > On 2018-11-26 19:13:17 -0500, Stephen Frost wrote: > > * Andres Freund (andres@anarazel.de) wrote: > > > On 2018-11-26 19:05:02 -0500, Stephen Frost wrote: > > > > Agreed, but I could see us having a regression test which complains if > > > > it finds any which are marked as immutable but aren't parallel safe. > > > > > > That doesn't help if a user writes a query to review the not parallel > > > safe functions in their installation. > > > > I'm really not sure what you're getting at here..? > > You replied to me saying it'd be a bad idea to infer parallel safety > from immutability: I don't think we should programatically assume it, but given that it's very likely the case, we should have a check (as you suggested in the other thread) that makes PG developers at least think about it. > > > > Surely a simple rule could be made that provolatile='i' trumps > > > > proparallel. No need to make them agree. > > > > [...] > > > > > > I think it'd be entirely unacceptable that > > > SELECT * FROM pg_proc WHERE proparallel = 's' > > > wouldn't actually return all the functions that are parallel safe. > > > > Agreed, but I could see us having a regression test which complains if > > it finds any which are marked as immutable but aren't parallel safe. > > I'm saying that that doesn't address my concern. Then I'm unclear as to what the concern here is..? > > Parallel safe functions should be marked as such. Immutable functions > > should be marked as such. We should not assume that one implies the > > other, nor should we operate as if they do. > > > > My suggestion for a regression test was to make PG developers really > > think about if their new immutable functions should also be marked as > > parallel safe, in the event that they forget to mark it as such. If > > it's really immutable and not parallel safe, then they need to adjust > > the expected regression test output (and we can all see it...). > > See http://archives.postgresql.org/message-id/20181126234521.rh3grz7aavx2ubjv%40alap3.anarazel.de Right, so you're also suggestion we have a regression test for it.. I'm starting to wonder if maybe we're in violent agreement here..? Thanks! Stephen
Attachment
On 27/11/2018 01:14, Stephen Frost wrote: > Greetings, > > * Vik Fearing (vik.fearing@2ndquadrant.com) wrote: >> On 27/11/2018 01:10, Stephen Frost wrote: >>> * Vik Fearing (vik.fearing@2ndquadrant.com) wrote: >>>> On 27/11/2018 01:05, Stephen Frost wrote: >>>>> That said, I do *not* think we should make any assumptions here- users >>>>> incorrectly mark things all the time but we shouldn't encourage that and >>>>> we shouldn't assume that functions marked as immutable are parallel >>>>> safe. >>>> >>>> Does that mean we also shouldn't assume that functions marked as >>>> immutable are index safe? >>> >>> We've got an index safe flag? >> >> Yes. It's called provolatile='i'. > > ... and we complain if someone tries to use a provolatile <> 'i' > function directly in an index, so not sure what you're getting at here? I'm getting at we should do the same for parallel safety checks. If it's immutable, it's safe. If it's not immutable, check if it's safe. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachment
On 27/11/2018 01:13, Stephen Frost wrote: > Parallel safe functions should be marked as such. Immutable functions > should be marked as such. We should not assume that one implies the > other, nor should we operate as if they do. Yes we should! Unless you can produce a case where an immutable function is not parallel safe. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachment
Hi, On 2018-11-27 01:27:41 +0100, Vik Fearing wrote: > On 27/11/2018 01:13, Stephen Frost wrote: > > Parallel safe functions should be marked as such. Immutable functions > > should be marked as such. We should not assume that one implies the > > other, nor should we operate as if they do. > > Yes we should! Unless you can produce a case where an immutable > function is not parallel safe. Well, then write a patch that documents that behaviour, automatically sets proparallel accordingly, and defines an escape hatch for when the user actually meant unsafe, not just "unsafe maybe". Just saying "we should" doesn't go far actually convincing anybody. Greetings, Andres Freund
FWIW, I'm inclined to think that pg_config should be marked as stable
not immutable. Aside from the minor-version-upgrade issue, what if
you install new binaries that are the same version but built with
different configure flags?
The pg_config outputs are roughly as stable as initdb-inserted
catalog data, and we've never judged that functions that return
catalog data can be marked immutable.
There seems no reason it can't be parallel safe, though, since
worker processes should get the same answers as their parent.
regards, tom lane
Greetings, * Vik Fearing (vik.fearing@2ndquadrant.com) wrote: > On 27/11/2018 01:13, Stephen Frost wrote: > > Parallel safe functions should be marked as such. Immutable functions > > should be marked as such. We should not assume that one implies the > > other, nor should we operate as if they do. > > Yes we should! Unless you can produce a case where an immutable > function is not parallel safe. What's the advantage of running a function that isn't marked as parallel safe in a parallel worker because it's marked as immutable? Seems like the only thing we're doing is making assumptions about what the user meant when they've clearly told us something different and that just strikes me as both a bad idea and an unnecessary complication of the code. Thanks! Stephen
Attachment
Vik Fearing <vik.fearing@2ndquadrant.com> writes:
> On 27/11/2018 01:13, Stephen Frost wrote:
>> Parallel safe functions should be marked as such. Immutable functions
>> should be marked as such. We should not assume that one implies the
>> other, nor should we operate as if they do.
> Yes we should! Unless you can produce a case where an immutable
> function is not parallel safe.
As far as that goes, I agree with the idea of adding an oprsanity test
that shows any built-in functions that are immutable but not parallel
safe, on the grounds Stephen mentioned: it's probably a mistake, and
if it isn't, we can add that function to the expected output.
I'm way less inclined to buy into the idea that it MUST be wrong, though.
Immutability is a promise about result stability and lack of side effects,
but it is not a promise about implementation details. There could be an
implementation reason not to run something in a parallel worker. Off the
top of my head, a possible example is "it's written in plfoo which hasn't
yet been made to work correctly in parallel workers".
regards, tom lane
On 27/11/2018 01:40, Tom Lane wrote: > Vik Fearing <vik.fearing@2ndquadrant.com> writes: >> On 27/11/2018 01:13, Stephen Frost wrote: >>> Parallel safe functions should be marked as such. Immutable functions >>> should be marked as such. We should not assume that one implies the >>> other, nor should we operate as if they do. > >> Yes we should! Unless you can produce a case where an immutable >> function is not parallel safe. > > As far as that goes, I agree with the idea of adding an oprsanity test > that shows any built-in functions that are immutable but not parallel > safe, on the grounds Stephen mentioned: it's probably a mistake, and > if it isn't, we can add that function to the expected output. > > I'm way less inclined to buy into the idea that it MUST be wrong, though. > Immutability is a promise about result stability and lack of side effects, > but it is not a promise about implementation details. There could be an > implementation reason not to run something in a parallel worker. Off the > top of my head, a possible example is "it's written in plfoo which hasn't > yet been made to work correctly in parallel workers". Now, see, that is an actual argument for making a difference. The other arguments in this thread were not, so say I. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
I'm a bit more concerned by the fact that inlining the function isn't affecting the parallel safety of the query - the fact that parallel safety is being checked prior to inlining means that if inlining *introduces* a parallel hazard, it will go unnoticed? -- Andrew (irc:RhodiumToad)
On Mon, Nov 26, 2018 at 7:47 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote: > > I'm way less inclined to buy into the idea that it MUST be wrong, though. > > Immutability is a promise about result stability and lack of side effects, > > but it is not a promise about implementation details. There could be an > > implementation reason not to run something in a parallel worker. Off the > > top of my head, a possible example is "it's written in plfoo which hasn't > > yet been made to work correctly in parallel workers". > > Now, see, that is an actual argument for making a difference. The other > arguments in this thread were not, so say I. I agree with you that Tom is the first person to make a real argument for distinguishing these two things. And I think his argument is a good one. I suspect that there are other cases too. I don't think there are all that many cases, but I think they exist. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, Nov 26, 2018 at 8:49 PM Andrew Gierth <andrew@tao11.riddles.org.uk> wrote: > I'm a bit more concerned by the fact that inlining the function isn't > affecting the parallel safety of the query - the fact that parallel > safety is being checked prior to inlining means that if inlining > *introduces* a parallel hazard, it will go unnoticed? If a function is marked parallel-safe but internally calls parallel-restricted or parallel-unsafe functions, it wasn't really parallel-safe in the first place. So I think that if inlining introduces a parallel hazard, the user has mislabeled some functions and any resulting injury is self-inflicted. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
>>>>> "Robert" == Robert Haas <robertmhaas@gmail.com> writes: >> I'm a bit more concerned by the fact that inlining the function >> isn't affecting the parallel safety of the query - the fact that >> parallel safety is being checked prior to inlining means that if >> inlining *introduces* a parallel hazard, it will go unnoticed? Robert> If a function is marked parallel-safe but internally calls Robert> parallel-restricted or parallel-unsafe functions, it wasn't Robert> really parallel-safe in the first place. So I think that if Robert> inlining introduces a parallel hazard, the user has mislabeled Robert> some functions and any resulting injury is self-inflicted. But the combination of inlining and polymorphism, in particular, makes it impossible for the function author to know this. Take the OP's example; it is parallel safe if and only if the selected type's equal function is parallel safe - how is the author supposed to know? What if the type is one installed later? -- Andrew (irc:RhodiumToad)
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Nov 26, 2018 at 8:49 PM Andrew Gierth
> <andrew@tao11.riddles.org.uk> wrote:
>> I'm a bit more concerned by the fact that inlining the function isn't
>> affecting the parallel safety of the query - the fact that parallel
>> safety is being checked prior to inlining means that if inlining
>> *introduces* a parallel hazard, it will go unnoticed?
> If a function is marked parallel-safe but internally calls
> parallel-restricted or parallel-unsafe functions, it wasn't really
> parallel-safe in the first place. So I think that if inlining
> introduces a parallel hazard, the user has mislabeled some functions
> and any resulting injury is self-inflicted.
Hm. We intentionally allow SQL functions to be marked as less mutable
than their internals, because sometimes that's useful for tricking
the planner. However, IIRC we don't inline when we see that's the
case. Maybe there needs to be a similar restriction for parallelism
markings.
Alternatively, could we postpone the parallelism checks till after
function inlining? Do we even make any before that?
regards, tom lane
On Mon, Nov 26, 2018 at 11:20 PM Andrew Gierth <andrew@tao11.riddles.org.uk> wrote: > But the combination of inlining and polymorphism, in particular, makes > it impossible for the function author to know this. Take the OP's > example; it is parallel safe if and only if the selected type's equal > function is parallel safe - how is the author supposed to know? What if > the type is one installed later? I think you have to set it conservatively. It's easy to construct all kinds of cases where a function is sometimes parallel-safe and sometimes not depending on the parameters passed to it, but we don't have any way to indicate that right now -- and I'm not entirely convinced that we need one. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: Tom> Hm. We intentionally allow SQL functions to be marked as less Tom> mutable than their internals, because sometimes that's useful for Tom> tricking the planner. However, IIRC we don't inline when we see Tom> that's the case. Maybe there needs to be a similar restriction for Tom> parallelism markings. Well, that doesn't help since not inlining the function doesn't prevent it from being called in parallel mode. On second thoughts the problem actually isn't directly to do with inlining at all. The problem is that in the presence of polymorphism, the function author can't have any confidence in setting any function as parallel-safe, since they can't know what the actual functions are that will be called at run time. The _nice_ fix for this would be to make it ok to leave inlinable functions as parallel-unsafe, and have the inlined version checked for parallel safety. Which would require doing... Tom> Alternatively, could we postpone the parallelism checks till after Tom> function inlining? Do we even make any before that? Right now, the parallelism checks are pretty much the very first thing done on the query, right at the start of standard_planner. -- Andrew (irc:RhodiumToad)
On Mon, Nov 26, 2018 at 11:21 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Hm. We intentionally allow SQL functions to be marked as less mutable > than their internals, because sometimes that's useful for tricking > the planner. However, IIRC we don't inline when we see that's the > case. Maybe there needs to be a similar restriction for parallelism > markings. Not sure what you have in mind here exactly. I think that the only correct thing to do is to mark the parent with the least safe setting that could apply. When you inline, you might find out that things are safer than they initially looked, but I think by then it's too late to change your mind because ... > Alternatively, could we postpone the parallelism checks till after > function inlining? Do we even make any before that? ... I believe the parallel-safety checks are done very early, and if you decide that it's not safe to proceed with parallelism, you can't really change your mind later. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Nov 26, 2018 at 11:21 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Alternatively, could we postpone the parallelism checks till after
>> function inlining? Do we even make any before that?
> ... I believe the parallel-safety checks are done very early, and if
> you decide that it's not safe to proceed with parallelism, you can't
> really change your mind later.
What do you consider "very early"? I do not offhand see a good reason
why we would need to know that before entering query_planner. Before
that, by definition, we have not made any paths.
regards, tom lane
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
> Tom> Hm. We intentionally allow SQL functions to be marked as less
> Tom> mutable than their internals, because sometimes that's useful for
> Tom> tricking the planner. However, IIRC we don't inline when we see
> Tom> that's the case. Maybe there needs to be a similar restriction for
> Tom> parallelism markings.
> Well, that doesn't help since not inlining the function doesn't prevent
> it from being called in parallel mode.
In the mutability case, our take on this is "you lied, so it's your
problem if you get the wrong answer". I'd be inclined to take the
same view in the parallelism case, except for the possibility that
forcing a parallel-unsafe function to be executed in a parallel
worker could lead to a crash. (This gets back to my point about
implementation details --- we don't know exactly why a function
is marked parallel unsafe, and it could be that it really positively
Doesn't Work in a worker, whereas there's not a similar issue with
respect to not-as-immutable-as-claimed functions.)
However ... if that's possible, then we have a security-ish issue
here, because it's not hard at all to ensure that a SQL function
*won't* be inlined. So somebody who was intent on crashing a
parallel worker could easily exploit a vulnerable function, inlining
or no: just wrap it in a mislabeled SQL wrapper.
Maybe the only real answer is that we need higher standards about
how badly a parallel-unsafe function can mess up. Or a lower-level
test that prevents one from being called in a worker, independently
of what the planner thought.
> On second thoughts the problem actually isn't directly to do with
> inlining at all. The problem is that in the presence of polymorphism,
> the function author can't have any confidence in setting any function as
> parallel-safe, since they can't know what the actual functions are that
> will be called at run time.
We've encountered similar issues with respect to the mutability of
datatype I/O functions, and resolved them via a messy compromise:
I/O functions aren't allowed to be volatile, while callers aren't
allowed to assume they're immutable. Not sure if such a concept
would help here.
regards, tom lane