From 24a2c6664e38448f652a17b96d16ccf58bcf4643 Mon Sep 17 00:00:00 2001 From: "Andrey V. Lepikhov" Date: Tue, 17 May 2022 17:48:49 +0500 Subject: [PATCH] Safe calculation of the minimum number of groups from couple of a long and a double values. Previous implementation returned negative value in the case of a double > LONG_MAX value. --- src/backend/executor/nodeSubplan.c | 3 +- src/backend/optimizer/plan/createplan.c | 8 +++-- src/test/regress/expected/aggregates.out | 41 ++++++++++++++++++++++++ src/test/regress/sql/aggregates.sql | 12 +++++++ 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c index 60d2290030..89dfe09970 100644 --- a/src/backend/executor/nodeSubplan.c +++ b/src/backend/executor/nodeSubplan.c @@ -498,7 +498,8 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext) node->havehashrows = false; node->havenullrows = false; - nbuckets = (long) Min(planstate->plan->plan_rows, (double) LONG_MAX); + nbuckets = (planstate->plan->plan_rows < (double) LONG_MAX) ? + (long) planstate->plan->plan_rows : LONG_MAX; if (nbuckets < 1) nbuckets = 1; diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index f4cc56039c..fb94d27473 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -2724,7 +2724,8 @@ 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 = (best_path->numGroups < (double) LONG_MAX) ? + (long) best_path->numGroups : LONG_MAX; plan = make_setop(best_path->cmd, best_path->strategy, @@ -2761,7 +2762,8 @@ 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 = (best_path->numGroups < (double) LONG_MAX) ? + (long) best_path->numGroups : LONG_MAX; plan = make_recursive_union(tlist, leftplan, @@ -6554,7 +6556,7 @@ make_agg(List *tlist, List *qual, long numGroups; /* Reduce to long, but 'ware overflow! */ - numGroups = (long) Min(dNumGroups, (double) LONG_MAX); + numGroups = dNumGroups < (double) LONG_MAX ? (long) dNumGroups : LONG_MAX; node->aggstrategy = aggstrategy; node->aggsplit = aggsplit; diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index 601047fa3d..0037981fc4 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -3049,10 +3049,51 @@ set work_mem to default; ----+----+---- (0 rows) +CREATE TABLE agg_group_5 AS ( + SELECT 1 AS x, NULL AS x1 FROM generate_series(1,1e5) x); +ANALYZE agg_group_5; +-- Stress test for grouping: exceed number of 2^53 groups to check +-- long <-> double conversion. +EXPLAIN (COSTS OFF) SELECT t1.x1 FROM + agg_group_5 t1,agg_group_5 t2,agg_group_5 t3,agg_group_5 t4,agg_group_5 t5, + agg_group_5 t6,agg_group_5 t7,agg_group_5 t8,agg_group_5 t9 +GROUP BY (t1.x1,t2.x1,t3.x1,t4.x1,t5.x1,t6.x1,t7.x1,t8.x1,t9.x1); + QUERY PLAN +---------------------------------------------------------------------------------------- + HashAggregate + Group Key: t1.x1, t2.x1, t3.x1, t4.x1, t5.x1, t6.x1, t7.x1, t8.x1, t9.x1 + -> Nested Loop + -> Nested Loop + -> Nested Loop + -> Nested Loop + -> Nested Loop + -> Nested Loop + -> Nested Loop + -> Nested Loop + -> Seq Scan on agg_group_5 t1 + -> Materialize + -> Seq Scan on agg_group_5 t2 + -> Materialize + -> Seq Scan on agg_group_5 t3 + -> Materialize + -> Seq Scan on agg_group_5 t4 + -> Materialize + -> Seq Scan on agg_group_5 t5 + -> Materialize + -> Seq Scan on agg_group_5 t6 + -> Materialize + -> Seq Scan on agg_group_5 t7 + -> Materialize + -> Seq Scan on agg_group_5 t8 + -> Materialize + -> Seq Scan on agg_group_5 t9 +(27 rows) + drop table agg_group_1; drop table agg_group_2; drop table agg_group_3; drop table agg_group_4; +drop table agg_group_5; drop table agg_hash_1; drop table agg_hash_2; drop table agg_hash_3; diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql index c6e0d7ba2b..e19d74fef5 100644 --- a/src/test/regress/sql/aggregates.sql +++ b/src/test/regress/sql/aggregates.sql @@ -1351,10 +1351,22 @@ set work_mem to default; union all (select * from agg_group_4 except select * from agg_hash_4); +CREATE TABLE agg_group_5 AS ( + SELECT 1 AS x, NULL AS x1 FROM generate_series(1,1e5) x); +ANALYZE agg_group_5; + +-- Stress test for grouping: exceed number of 2^53 groups to check +-- long <-> double conversion. +EXPLAIN (COSTS OFF) SELECT t1.x1 FROM + agg_group_5 t1,agg_group_5 t2,agg_group_5 t3,agg_group_5 t4,agg_group_5 t5, + agg_group_5 t6,agg_group_5 t7,agg_group_5 t8,agg_group_5 t9 +GROUP BY (t1.x1,t2.x1,t3.x1,t4.x1,t5.x1,t6.x1,t7.x1,t8.x1,t9.x1); + drop table agg_group_1; drop table agg_group_2; drop table agg_group_3; drop table agg_group_4; +drop table agg_group_5; drop table agg_hash_1; drop table agg_hash_2; drop table agg_hash_3; -- 2.36.0