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:

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

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.

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.

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.

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,

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

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

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

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

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

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

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

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

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