On Mon, Jun 27, 2016 at 6:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Mon, Jun 27, 2016 at 5:28 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Seems to me that it should generally be the case that consider_parallel
>>> would already be clear on the parent rel if the tlist isn't parallel safe,
>>> and if it isn't we probably have a bug elsewhere. If it makes you feel
>>> better, maybe you could add Assert(!has_parallel_hazard(...)) here?
>
>> I don't see that this is true. If someone does SELECT
>> pg_backend_pid() FROM pgbench_accounts, there's only one RelOptInfo
>> and nothing to clear consider_parallel for it anywhere else.
>
> Huh? The final tlist would go with the final_rel, ISTM, not the scan
> relation. Maybe we have some rejiggering to do to make that true, though.
Mumble. You're right that there are two rels involved, but I think
I'm still right about the substance of the problem. I can't tell
whether the remainder of your email concedes that point or whether
we're still in disagreement.
In that example, SELECT pg_backend_pid() FROM pgbench_accounts, we're
first going to form a path for scanning pgbench_accounts. The rel for
pgbench_accounts will be marked parallel_safe because it's just a scan
of a relation outputting some number (possibly 0) of Vars. That rel
becomes the final scan/join rel, and the path or paths for that rel
are parallel-safe. Now, when we apply the final tlist to those paths,
they are no longer parallel-safe. apply_projection_to_path() has got
to realize that.
> You could still save something by writing code along the line of
> if (path->parallel_safe &&
> has_parallel_hazard(...))
> path->parallel_safe = false;
> so as not to run has_parallel_hazard in the case where we already know
> we lost.
I agree, and that does seem worth doing.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company