Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-16 Thread Tom Lane
Robert Haas writes: > You want to push something, or should I do it? Go for it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-16 Thread Robert Haas
On Wed, Aug 16, 2017 at 12:49 PM, Tom Lane wrote: > Robert Haas writes: >> On Wed, Aug 16, 2017 at 11:56 AM, Tom Lane wrote: >>> I can get on board with that statement. Can you draft a better wording? > >> Here is an attempt. Feel free to edit. > > I think s/plan/query/ in the last bit would b

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-16 Thread Tom Lane
Robert Haas writes: > On Wed, Aug 16, 2017 at 11:56 AM, Tom Lane wrote: >> I can get on board with that statement. Can you draft a better wording? > Here is an attempt. Feel free to edit. I think s/plan/query/ in the last bit would be better. Perhaps +* However, if force_parallel_mo

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-16 Thread Robert Haas
On Wed, Aug 16, 2017 at 11:56 AM, Tom Lane wrote: >> So I still don't see what's wrong here, other than that the comment is >> evidently not half clear enough. > > I can get on board with that statement. Can you draft a better wording? Here is an attempt. Feel free to edit. -- Robert Haas Ent

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-16 Thread Tom Lane
Robert Haas writes: > On Wed, Aug 16, 2017 at 11:03 AM, Tom Lane wrote: >> Well, it has subplans, so formally I think it's restricted not unsafe >> --- but the parallel_safe marking on constructed paths/plans is only >> safe/not-safe, not a three-way. > True, but when parallel_safe it not set, t

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-16 Thread Robert Haas
On Wed, Aug 16, 2017 at 11:03 AM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Aug 15, 2017 at 6:40 PM, Tom Lane wrote: >>> (In fact, a quick look shows a counterexample: if we pick a MinMaxAgg >>> path, that's parallel unsafe, but the original query might've been >>> completely safe.) > >>

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-16 Thread Tom Lane
Robert Haas writes: > On Tue, Aug 15, 2017 at 6:40 PM, Tom Lane wrote: >> (In fact, a quick look shows a counterexample: if we pick a MinMaxAgg >> path, that's parallel unsafe, but the original query might've been >> completely safe.) > I'm quite confused here. What's parallel-unsafe about a Mi

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-15 Thread Robert Haas
On Tue, Aug 15, 2017 at 6:40 PM, Tom Lane wrote: > No. The case that I'm concerned about is where the initial estimate > of "parallelModeOK" is true, but the planner nevertheless selects > a parallel-unsafe plan --- unsafe for some other reason than that it > already has a Gather in it. That wou

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-15 Thread Tom Lane
I wrote: > As I said, I think this code is based on fuzzy thinking. ... or, more charitably, it may just date from before standard_planner's plaster-a-Gather-on-top logic worked the way it does today. But in any case I think it's wrong now. regards, tom lane -- Sent vi

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-15 Thread Tom Lane
Robert Haas writes: > On Tue, Aug 15, 2017 at 5:15 PM, Tom Lane wrote: >> The idea of the existing >> code seems to be "let's exercise what happens if the executor does >> EnterParallelMode/ExitParallelMode around any plan whatsoever, even a >> parallel-unsafe one"; which seems to me to be bogus

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-15 Thread Robert Haas
On Tue, Aug 15, 2017 at 5:15 PM, Tom Lane wrote: > The idea of the existing > code seems to be "let's exercise what happens if the executor does > EnterParallelMode/ExitParallelMode around any plan whatsoever, even a > parallel-unsafe one"; which seems to me to be bogus as heck. I think that is n

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-15 Thread Tom Lane
I wrote: > I like your plan (2). It's not much code and it lends itself to having a > run-time check, rather than just an Assert, that we found a Result node. > That seems like a good idea now that we've found the assumption isn't > bulletproof. However, do we need to worry about the planner putt

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-15 Thread Tom Lane
Robert Haas writes: > On Tue, Aug 15, 2017 at 2:34 PM, Tom Lane wrote: >> I assume (haven't looked) that I could hack the plpgsql code to prevent >> generating a parallel plan when it's decided the command is a simple >> SELECT. But I wonder whether that's the right place to fix it. Does >> it

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-15 Thread Robert Haas
On Tue, Aug 15, 2017 at 2:34 PM, Tom Lane wrote: > Tom Lane writes: >> Simplify plpgsql's check for simple expressions. >> ... >> https://git.postgresql.org/pg/commitdiff/00418c61244138bd8ac2de58076a1d0dd4f539f3 > > The buildfarm members that are running force_parallel_mode = regress > are not ha

Re: [HACKERS] [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

2017-08-15 Thread Tom Lane
Tom Lane writes: > Simplify plpgsql's check for simple expressions. > ... > https://git.postgresql.org/pg/commitdiff/00418c61244138bd8ac2de58076a1d0dd4f539f3 The buildfarm members that are running force_parallel_mode = regress are not happy with this. Apparently, even a trivial SELECT can be tu