Thread: pgsql: Don't generate parallel paths for rels with parallel-restricted

Don't generate parallel paths for rels with parallel-restricted outputs.

Such paths are unsafe.  To make it cheaper to detect when this case
applies, track whether a relation's default PathTarget contains any
non-Vars.  In most cases, the answer will be no, which enables us to
determine cheaply that the target list for a proposed path is
parallel-safe.  However, subquery pull-up can create cases that
require us to inspect the target list more carefully.

Amit Kapila, reviewed by me.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/b12fd41c695b43c76b0a9a4d19ba43b05536440c

Modified Files
--------------
src/backend/nodes/outfuncs.c             |  1 +
src/backend/optimizer/path/allpaths.c    | 10 ++++++++++
src/backend/optimizer/util/placeholder.c |  2 ++
src/backend/optimizer/util/relnode.c     | 10 +++++++---
src/include/nodes/relation.h             |  2 ++
5 files changed, 22 insertions(+), 3 deletions(-)

Robert Haas <rhaas@postgresql.org> writes:
> Don't generate parallel paths for rels with parallel-restricted outputs.

Surely this bit is wrong on its face:

@@ -971,6 +980,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
            adjust_appendrel_attrs(root,
                                   (Node *) rel->reltarget->exprs,
                                   appinfo);
+       childrel->reltarget_has_non_vars = rel->reltarget_has_non_vars;

        /*
         * We have to make child entries in the EquivalenceClass data

The entire point of what we are doing here is that Vars might get replaced
with things that are not Vars.  See the comment a dozen lines above.

More generally, if we need such a flag (which I doubt really), perhaps
it should be part of PathTarget rather than expecting that it can
successfully be maintained separately?

It seems like the only reason that we would need such a flag is that
applying has_parallel_hazard() to a Var is a bit expensive thanks to
the typeid_is_temp() test --- but if you ask me, that test is stupid
and should be removed.  What's the argument for supposing that a temp
table's rowtype is any more mutable intra-query than any other rowtype?

            regards, tom lane