On Thu, Jun 30, 2016 at 6:49 PM, Robert Haas <robertmh...@gmail.com> wrote:
> I haven't had a chance to do this yet, so I'm going to do it tomorrow instead.

I dug into this a bit and found more problems.  I wondered why Tom's
patch did this:

!                       if (has_parallel_hazard((Node *) rte->subquery, false))
!                               return;
!                       break;

Instead of just doing this:

-            return;
+            break;

After all, the code that built subquery paths ought to be sufficient
to find any parallel hazards during subquery planning; there seems to
be no especially-good reason why we should walk the whole query tree
searching for the again.  So I changed that and ran the regression
tests.  With force_parallel_mode=on, things got unhappy on exactly one
query:

  SELECT * FROM
  (SELECT a || b AS ab FROM t1
   UNION ALL
   SELECT ab FROM t2) t
  ORDER BY 1 LIMIT 8;

This failed with a complaint about parallel workers trying to touch
temporary relations, which turns out to be pretty valid since t1 and
t2 are BOTH temporary relations.  This turns out to be another facet
of my ignorance about how subqueries can be pulled up to create
appendrels with crazy things in them.  set_rel_consider_parallel()
looks at the appendrel and thinks everything is fine, because the
reference to temporary tables are buried inside the appendrel members,
which are note examined.

I think it's hard to avoid the conclusion that
set_rel_consider_parallel() needs to run on other member rels as well
as baserels.  We might be able to do that only for cases where the
parent is a subquery RTE, since if the parent is a baserel then I
think we must have just a standard inheritance hierarchy and things
might be OK, but even then, I fear there might be problems with RLS.
Anyway, the attached patch solves the problem in a fairly "brute
force" fashion.  We loop over all basrels and other member rels and
set consider_parallel according to their properties.  Then, when
setting base rel sizes, we clear consider_parallel for any parents if
it's clear for any of the children.  Finally, before setting base rel
pathlists for appendrel children, we clear the flag for the child if
it's meanwhile been cleared for the parent.  Thus, the parents and
children always agree and only consider parallel paths for any of the
rels if they're all OK.  This seems a bit grotty to me so suggestions
are welcome.

(Official status update: I'm not prepared to commit this without some
review.  So I intend to wait for a while and see whether I get some
review.  I realize these status updates are supposed to contain a date
by which further action will be taken, but I don't know how to
meaningfully do that in this type of case.)

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

Attachment: more-allpaths-fixes-v1.patch
Description: invalid/octet-stream

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

Reply via email to