diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2022-05-21 13:13:41 -0400 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2022-05-21 13:13:44 -0400 |
| commit | a916cb9d5a89804998dd4e7fd7bbb27cb5a7abc8 (patch) | |
| tree | 95fff229b2b404be908ad28ff2f9406aa1e3f497 /src/backend/optimizer/plan/createplan.c | |
| parent | ac1ae477f85c6aeb3119071c1c00eb042b4afa4d (diff) | |
| download | postgresql-a916cb9d5a89804998dd4e7fd7bbb27cb5a7abc8.tar.gz postgresql-a916cb9d5a89804998dd4e7fd7bbb27cb5a7abc8.zip | |
Avoid overflow hazard when clamping group counts to "long int".
Several places in the planner tried to clamp a double value to fit
in a "long" by doing
(long) Min(x, (double) LONG_MAX);
This is subtly incorrect, because it casts LONG_MAX to double and
potentially back again. If long is 64 bits then the double value
is inexact, and the platform might round it up to LONG_MAX+1
resulting in an overflow and an undesirably negative output.
While it's not hard to rewrite the expression into a safe form,
let's put it into a common function to reduce the risk of someone
doing it wrong in future.
In principle this is a bug fix, but since the problem could only
manifest with group count estimates exceeding 2^63, it seems unlikely
that anyone has actually hit this or will do so anytime soon. We're
fixing it mainly to satisfy fuzzer-type tools. That being the case,
a HEAD-only fix seems sufficient.
Andrey Lepikhov
Discussion: https://postgr.es/m/ebbc2efb-7ef9-bf2f-1ada-d6ec48f70e58@postgrespro.ru
Diffstat (limited to 'src/backend/optimizer/plan/createplan.c')
| -rw-r--r-- | src/backend/optimizer/plan/createplan.c | 7 |
1 files changed, 3 insertions, 4 deletions
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index f4cc56039c2..76606faa3e4 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -16,7 +16,6 @@ */ #include "postgres.h" -#include <limits.h> #include <math.h> #include "access/sysattr.h" @@ -2724,7 +2723,7 @@ create_setop_plan(PlannerInfo *root, SetOpPath *best_path, int flags) flags | CP_LABEL_TLIST); /* Convert numGroups to long int --- but 'ware overflow! */ - numGroups = (long) Min(best_path->numGroups, (double) LONG_MAX); + numGroups = clamp_cardinality_to_long(best_path->numGroups); plan = make_setop(best_path->cmd, best_path->strategy, @@ -2761,7 +2760,7 @@ create_recursiveunion_plan(PlannerInfo *root, RecursiveUnionPath *best_path) tlist = build_path_tlist(root, &best_path->path); /* Convert numGroups to long int --- but 'ware overflow! */ - numGroups = (long) Min(best_path->numGroups, (double) LONG_MAX); + numGroups = clamp_cardinality_to_long(best_path->numGroups); plan = make_recursive_union(tlist, leftplan, @@ -6554,7 +6553,7 @@ make_agg(List *tlist, List *qual, long numGroups; /* Reduce to long, but 'ware overflow! */ - numGroups = (long) Min(dNumGroups, (double) LONG_MAX); + numGroups = clamp_cardinality_to_long(dNumGroups); node->aggstrategy = aggstrategy; node->aggsplit = aggsplit; |