Robert Haas <robertmh...@gmail.com> writes: > On Tue, Aug 15, 2017 at 5:15 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> The idea of the existing >> code seems to be "let's exercise what happens if the executor does >> EnterParallelMode/ExitParallelMode around any plan whatsoever, even a >> parallel-unsafe one"; which seems to me to be bogus as heck.
> I think that is not what the current code is doing. If the plan is > diagnosed as parallel-unsafe, then parallelModeOK will be false and > nothing will happen. No. The case that I'm concerned about is where the initial estimate of "parallelModeOK" is true, but the planner nevertheless selects a parallel-unsafe plan --- unsafe for some other reason than that it already has a Gather in it. That would prevent the code further down in standard_planner from plastering a Gather on top, but we still end up labeling the output plan with parallelModeNeeded = true. Now, you might argue that there should be no case where that initial estimate has parallelModeOK = true and yet we end up with a parallel-unsafe plan. I do not think I believe that; that estimate is supposed to be a conservative estimate, not ironclad exactness. (In fact, a quick look shows a counterexample: if we pick a MinMaxAgg path, that's parallel unsafe, but the original query might've been completely safe.) > If the plan is actually parallel-unsafe but the > planner doesn't *think* it's parallel-unsafe, then what you are > talking about will happen, but that seems to me to be a good thing. No, you have that backwards. If the planner incorrectly thinks the plan is parallel-safe then it will forcibly put a Gather on top, and we'll mark parallelModeNeeded = true due to the existing assignment where that is done, and then we will detect the unsafety at runtime. The case I'm worried about is where the planner knows (correctly) that the selected plan is parallel-unsafe, due to some choice made after the initial parallelModeOK = true estimate. > It lets you find planner bugs (or functions that a user has labelled > incorrectly). Don't believe this argument either. Certainly we want to be able to detect incorrectly-labeled-safe functions by turning on force_parallel_mode; but that will happen anyway, since both the initial parallelModeOK estimate and the final top_plan->parallel_safe flag will reflect the false safety labeling. (If there's something else in the plan that makes it parallel-unsafe overall, then the test cannot reveal the false function labeling anyway.) As I said, I think this code is based on fuzzy thinking. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers