On Mon, 19 Oct 2020 at 12:10, Tom Lane <t...@sss.pgh.pa.us> wrote: > > David Rowley <dgrowle...@gmail.com> writes: > > For the backbranches, I think I go with something more minimal in the > > form of adding: > > TBH, I see no need to do anything in the back branches. This is not > an issue for production usage.
I understand the Assert failure is pretty harmless, so non-assert builds shouldn't suffer too greatly. I just assumed that any large stakeholders invested in upgrading to a newer version of PostgreSQL may like to run various tests with their application against an assert enabled version of PostgreSQL perhaps to gain some confidence in the upgrade. A failing assert is unlikely to inspire additional confidence. I'm not set on backpatching, but that's just my thoughts. FWIW, the patch I'd thought of is attached. David
From a309f1f0dc0329e16e328d6c3766c4014a64319b Mon Sep 17 00:00:00 2001 From: "dgrow...@gmail.com" <dgrow...@gmail.com> Date: Mon, 19 Oct 2020 11:49:51 +1300 Subject: [PATCH] Fix Assert failure in join costing code In the planner, it was possible, given an extreme enough case containing a large number of joins for the number of estimated rows to become infinite. This was a problem in initial_cost_mergejoin() where the row estimate could be multiplied by 0 which resulted in NaN. This could cause the Asserts which verified the skip rows was <= rows to fail due to the fact that NaN <= Inf is false. To fix this, modify the join costing functions to look for isinf() row estimates and explicitly set those to DBL_MAX. It seems the various places which perform calculations on these values only multiply by a selectivity estimate, which should be between 0.0 and 1.0, so we should never end up with infinity again after the multiplication takes place. In the reported case, only initial_cost_mergejoin() suffered from the failing Assert(). However, both final_cost_nestloop() and final_cost_mergejoin() seem to make similar assumptions about the row estimates not being infinite, so change those too. This particular problem was fixed in master in a90c950fc, however, that fix seemed a little too generic to go backpatching, so this is the minimal fix for the same problem. Reported-by: Onder Kalaci Discussion: https://postgr.es/m/dm6pr21mb1211ff360183bca901b27f04d8...@dm6pr21mb1211.namprd21.prod.outlook.com --- src/backend/optimizer/path/costsize.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index f39e6a9f80..f1eb9194ba 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -71,6 +71,7 @@ #include "postgres.h" +#include <float.h> #include <math.h> #include "access/amapi.h" @@ -2737,11 +2738,18 @@ final_cost_nestloop(PlannerInfo *root, NestPath *path, QualCost restrict_qual_cost; double ntuples; - /* Protect some assumptions below that rowcounts aren't zero or NaN */ + /* + * Protect some assumptions below that rowcounts aren't zero, NaN or + * infinite. + */ if (outer_path_rows <= 0 || isnan(outer_path_rows)) outer_path_rows = 1; + else if (isinf(outer_path_rows)) + outer_path_rows = DBL_MAX; if (inner_path_rows <= 0 || isnan(inner_path_rows)) inner_path_rows = 1; + else if (isinf(inner_path_rows)) + inner_path_rows = DBL_MAX; /* Mark the path with the correct row estimate */ if (path->path.param_info) @@ -2952,11 +2960,18 @@ initial_cost_mergejoin(PlannerInfo *root, JoinCostWorkspace *workspace, innerendsel; Path sort_path; /* dummy for result of cost_sort */ - /* Protect some assumptions below that rowcounts aren't zero or NaN */ + /* + * Protect some assumptions below that rowcounts aren't zero, NaN or + * infinite. + */ if (outer_path_rows <= 0 || isnan(outer_path_rows)) outer_path_rows = 1; + else if (isinf(outer_path_rows)) + outer_path_rows = DBL_MAX; if (inner_path_rows <= 0 || isnan(inner_path_rows)) inner_path_rows = 1; + else if (isinf(inner_path_rows)) + inner_path_rows = DBL_MAX; /* * A merge join will stop as soon as it exhausts either input stream @@ -3185,9 +3200,14 @@ final_cost_mergejoin(PlannerInfo *root, MergePath *path, rescannedtuples; double rescanratio; - /* Protect some assumptions below that rowcounts aren't zero or NaN */ + /* + * Protect some assumptions below that rowcounts aren't zero, NaN or + * infinite. + */ if (inner_path_rows <= 0 || isnan(inner_path_rows)) inner_path_rows = 1; + else if (isinf(inner_path_rows)) + inner_path_rows = DBL_MAX; /* Mark the path with the correct row estimate */ if (path->jpath.path.param_info) -- 2.21.0.windows.1
