From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions. |
Date: | 2017-08-15 21:15:27 |
Message-ID: | 29231.1502831727@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
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 putting some
> nontrivial computation into the Gather's tlist instead of the Result?
I concluded that it'd probably be enough to have an assertion that the
Gather's tlist is trivial, so I made it work that way.
However, while investigating the behavior of force_parallel_mode along
the way to this, I found that standard_planner() contains some fuzzy
thinking about how to set parallelModeNeeded. It's not necessary or
(IMO) approriate to force that on just because of force_parallel_mode,
so I propose the attached patch, which deletes that stanza in favor of
initializing glob->parallelModeNeeded to just plain false. The effect of
this will be that parallelModeNeeded is only set true if there's actually
a Gather (or GatherMerge) node somewhere in the plan. The only case where
that differs from the existing behavior is if the initial checks conclude
that parallelModeOK can be turned on, but then we end up with a
parallel-unsafe plan for some reason or other. 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. If it failed,
we would not conclude that that was an executor bug.
So I think we should apply and back-patch the below.
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
disallow-setting-parallelModeNeeded-for-unsafe-plans.patch | text/x-diff | 1.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2017-08-15 22:08:47 | Re: [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions. |
Previous Message | Alvaro Herrera | 2017-08-15 21:14:37 | pgsql: Simplify autovacuum work-item implementation |
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2017-08-15 21:17:04 | Re: POC: Sharing record typmods between backends |
Previous Message | Jeff Janes | 2017-08-15 21:14:58 | Re: Create replication slot in pg_basebackup if requested and not yet present |