On Sun, Nov 29, 2020 at 4:10 PM Tomas Vondra <tomas.von...@enterprisedb.com> wrote: > > > > On 11/29/20 3:44 PM, James Coleman wrote: > > On Mon, Nov 23, 2020 at 8:19 AM James Coleman <jtc...@gmail.com> wrote: > >> > >> .. > >> > >> So I'm +1 on turning it into an ERROR log instead, since it seems to > >> me that encountering this case would almost certainly represent a bug > >> in path generation. > > > > Here's a patch to do that. > > > > Thanks. Isn't the comment incomplete? Should it mention just parallel > safety or also volatility?
Volatility makes it parallel unsafe, and I'm not sure I want to get into listing every possible issue here, but in the interest of making it more obviously representative of the kinds of issues we might encounter, I've tweaked it to mention volatility also, as well as refer to the issues as "examples" of such concerns. James
From b4b08b70d8961ea29587412e9a2ef4dd39111ff0 Mon Sep 17 00:00:00 2001 From: jcoleman <jtc...@gmail.com> Date: Sun, 29 Nov 2020 09:38:59 -0500 Subject: [PATCH v2] Error if gather merge paths aren't sufficiently sorted --- src/backend/optimizer/plan/createplan.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 40abe6f9f6..5ecf9f4065 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -1793,13 +1793,15 @@ create_gather_merge_plan(PlannerInfo *root, GatherMergePath *best_path) &gm_plan->nullsFirst); - /* Now, insert a Sort node if subplan isn't sufficiently ordered */ + /* + * All gather merge paths should have already guaranteed the necessary sort + * order either by adding an explicit sort node or by using presorted input. + * We can't simply add a sort here on additional pathkeys, because we can't + * guarantee the sort would be safe. For example, expressions may be + * volatile or otherwise parallel unsafe. + */ if (!pathkeys_contained_in(pathkeys, best_path->subpath->pathkeys)) - subplan = (Plan *) make_sort(subplan, gm_plan->numCols, - gm_plan->sortColIdx, - gm_plan->sortOperators, - gm_plan->collations, - gm_plan->nullsFirst); + elog(ERROR, "gather merge input not sufficiently sorted"); /* Now insert the subplan under GatherMerge. */ gm_plan->plan.lefttree = subplan; -- 2.17.1
