summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane2018-12-18 16:19:39 +0000
committerTom Lane2018-12-18 16:19:39 +0000
commit7c2bc40b8b78442d414db76366e6d15d2fa6ebd0 (patch)
tree4094af8b42d8ba531f1514e20770eebd3f0fe8ec
parent91c39daa3a4760f082a58133bb57b42b6965a5c3 (diff)
Fix ancient thinko in mergejoin cost estimation.
"rescanratio" was computed as 1 + rescanned-tuples / total-inner-tuples, which is sensible if it's to be multiplied by total-inner-tuples or a cost value corresponding to scanning all the inner tuples. But in reality it was (mostly) multiplied by inner_rows or a related cost, numbers that take into account the possibility of stopping short of scanning the whole inner relation thanks to a limited key range in the outer relation. This'd still make sense if we could expect that stopping short would result in a proportional decrease in the number of tuples that have to be rescanned. It does not, however. The argument that establishes the validity of our estimate for that number is independent of whether we scan all of the inner relation or stop short, and experimentation also shows that stopping short doesn't reduce the number of rescanned tuples. So the correct calculation is 1 + rescanned-tuples / inner_rows, and we should be sure to multiply that by inner_rows or a corresponding cost value. Most of the time this doesn't make much difference, but if we have both a high rescan rate (due to lots of duplicate values) and an outer key range much smaller than the inner key range, then the error can be significant, leading to a large underestimate of the cost associated with rescanning. Per report from Vijaykumar Jain. This thinko appears to go all the way back to the introduction of the rescan estimation logic in commit 70fba7043, so back-patch to all supported branches. Discussion: https://postgr.es/m/CAE7uO5hMb_TZYJcZmLAgO6iD68AkEK6qCe7i=vZUkCpoKns+EQ@mail.gmail.com
-rw-r--r--src/backend/optimizer/path/costsize.c11
1 files changed, 8 insertions, 3 deletions
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index d89b72e07a4..ff304e8624e 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -2491,8 +2491,13 @@ final_cost_mergejoin(PlannerInfo *root, MergePath *path,
if (rescannedtuples < 0)
rescannedtuples = 0;
}
- /* We'll inflate various costs this much to account for rescanning */
- rescanratio = 1.0 + (rescannedtuples / inner_path_rows);
+
+ /*
+ * We'll inflate various costs this much to account for rescanning. Note
+ * that this is to be multiplied by something involving inner_rows, or
+ * another number related to the portion of the inner rel we'll scan.
+ */
+ rescanratio = 1.0 + (rescannedtuples / inner_rows);
/*
* Decide whether we want to materialize the inner input to shield it from
@@ -2519,7 +2524,7 @@ final_cost_mergejoin(PlannerInfo *root, MergePath *path,
* of the generated Material node.
*/
mat_inner_cost = inner_run_cost +
- cpu_operator_cost * inner_path_rows * rescanratio;
+ cpu_operator_cost * inner_rows * rescanratio;
/*
* Prefer materializing if it looks cheaper, unless the user has asked to