Re: ERROR: ORDER/GROUP BY expression not found in targetlist - Mailing list pgsql-hackers

On Mon, Jun 13, 2016 at 4:46 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> In practice, we don't yet have the ability for
>> parallel-safe paths from subqueries to affect planning at higher query
>> levels, but that's because the pathification stuff landed too late in
>> the cycle for me to fully react to it.
>
> I wonder if that's not just from confusion between subplans and
> subqueries.  I don't believe any of the claims made in the comment
> adjusted in the patch below (other than "Subplans currently aren't passed
> to workers", which is true but irrelevant).  Nested Gather nodes is
> something that you would need, and already have, a defense for anyway.

I think you may be correct.

One problem that I've realized that is related to this is that the way
that the consider_parallel flag is being set for upper rels is almost
totally bogus, which I believe accounts for your complaint at PGCon
that force_parallel_mode was not doing as much as it ought to do.
When I originally wrote a lot of this logic, there were no upper rels,
which led to this code:

        if (force_parallel_mode == FORCE_PARALLEL_OFF || !glob->parallelModeOK)
        {
                glob->parallelModeNeeded = false;
                glob->wholePlanParallelSafe = false;    /* either
false or don't care */
        }
        else
        {
                glob->parallelModeNeeded = true;
                glob->wholePlanParallelSafe =
                        !has_parallel_hazard((Node *) parse, false);
        }

The main way that wholePlanParallelSafe is supposed to be cleared is
in create_plan:

        /* Update parallel safety information if needed. */
        if (!best_path->parallel_safe)
                root->glob->wholePlanParallelSafe = false;

However, at the time that code was written, it was impossible to rely
entirely on the latter mechanism.  Since there were no upper rels and
no paths for upper plan nodes, you could have the case where every
path was parallel-safe but the whole plan was node.  Therefore the
code shown above was needed to scan the whole darned plan for
parallel-unsafe things.  Post-pathification, this whole thing is
pretty bogus: upper rels mostly don't get consider_parallel set at
all, which means plans involving upper rels get marked parallel-unsafe
even if they are not, which means the wholePlanParallelSafe flag gets
cleared in a bunch of cases where it shouldn't.  And on the flip side
I think that the first chunk of code quoted above would be totally
unnecessary if we were actually setting consider_parallel correctly on
the upper rels.

I started working on a patch to fix all this, which I'm attaching here
so you can see what I'm thinking.  I am not sure it's correct, but it
does cause force_parallel_mode to do something interesting in many
more cases.

Anyway, the reason this is related to the issue at hand is that you
might have something like A LEFT JOIN (SELECT x, sum(q) FROM B GROUP
BY 1) ON A.x = B.x.   Right now, I think that the paths for the
aggregated subquery will always be marked as not parallel-safe, but
that's only because the consider_parallel flag on the grouped rel
isn't being set properly.  If it were, you could theoretically do a
parallel seq scan on A and then have each worker evaluate the join for
the rows that pop out of the subquery.  Maybe that's a stupid example,
but the point is that I bet there are cases where failing to mark the
upper rels with consider_parallel where appropriate causes good
parallel plans not to get generated.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company