pg.ddx.io pgsql-bugs@postgresql.org mailing list archive help / color / mirror / Atom feed From: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
To: pgsql-bugs@lists.postgresql.org
Subject: Re: Negative value of numGroups
Date: Tue, 17 May 2022 17:59:23 +0500
Message-ID: <ed1d1a2d-6061-24e5-e8eb-dbda3ec8f66c@postgrespro.ru> (raw )
In-Reply-To: <ebbc2efb-7ef9-bf2f-1ada-d6ec48f70e58@postgrespro.ru >
References: <ebbc2efb-7ef9-bf2f-1ada-d6ec48f70e58@postgrespro.ru >
On 13/5/2022 11:56, Andrey Lepikhov wrote:
> (long) Min(numGroups, (double) LONG_MAX);
>
> can return negative value, if numGroups > LONG_MAX.
So, because this problem still breaks sqlancer tests I prepared a patch
(with regression test) that fixes it.
--
regards,
Andrey Lepikhov
Postgres Professional
Attachments:
0001-Safe-calculation-of-the-minimum-number-of-groups-fro.patch (text/plain, 6.1K) download
From 24a2c6664e38448f652a17b96d16ccf58bcf4643 Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>
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
view thread (5+ messages) latest in thread
Message-ID: <ed1d1a2d-6061-24e5-e8eb-dbda3ec8f66c@postgrespro.ru>
Permalink: ../ed1d1a2d-6061-24e5-e8eb-dbda3ec8f66c@postgrespro.ru/
Also on: postgresql.org/message-id/ed1d1a2d-6061-24e5-e8eb-dbda3ec8f66c@postgrespro.ru
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: a.lepikhov@postgrespro.ru, pgsql-bugs@lists.postgresql.org
Subject: Re: Negative value of numGroups
In-Reply-To: <ed1d1a2d-6061-24e5-e8eb-dbda3ec8f66c@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox