On Mon, Jun 27, 2016 at 6:04 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Robert Haas <robertmh...@gmail.com> writes:
>> On Mon, Jun 27, 2016 at 5:28 PM, Tom Lane <t...@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

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to