On Fri, Aug 25, 2017 at 11:15 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > The problem I exhibited with the updated patch could probably be resolved > if the top level logic in a parallel worker knows about tuples_needed > and doesn't try to pull more than that from its subplan. So what you're > describing isn't just an additional optimization, it's necessary to make > this work at all.
I had the same thought. Done in the attached revision of your version. I couldn't consistently reproduce the failure you saw -- it failed for me only about 1 time in 10 -- but I don't think it's failing any more with this version. I think that the comment at the bottom of ExecSetTupleBound should probably be rethought a bit in light of these changes. Teaching the parallel worker about tuples_needed may not be JUST an additional optimization, but it IS an additional optimization; IOW, what the planner can put between Sort and Limit will no longer be the only relevant consideration. The details aside, Konstantin Knizhnik's proposal to teach this code about ForeignScans is yet another independent way for this to win, and I think that will also be quite a large win. In the future, I believe that we might want to use a mechanism like this in even more cases. For example, a Semi Join or Anti Join only needs 1 row from the inner side per execution. There's probably no reason to put a Sort under the inner side of a Semi Join or Anti Join, but it's certainly not impossible to have a Foreign Scan there. It's likely to be an expensive plan, but it's a heck of a lot more expensive if we fetch 100 rows when we only need 1. I'm not sure the existing mechanism will stretch to handle such cases. It's possible that we should work out some of these things at plan time rather than execution time, and it's also possible that a separate call to ExecSetTupleBound is too much work. I've mulled over the idea of whether we should get rid of this mechanism altogether in favor of passing the tuple bound as an additional argument to ExecProcNode, because it seems silly to call node-specific code once to set a (normally small, possibly 1) bound and then call it again to get a (or the) tuple. But I'm not sure passing it to ExecProcNode is really the right idea; it's adding some overhead for something that is a bit of a special case, and I'm not sure it will work out cleanly otherwise, either. I'd be interested in your thoughts if you have any. My intuition is that there's substantially more to be gained in this area, but exactly how best to gain it is not very clear to me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
push-down-bound-to-workers-3.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
