From: 狂奔的蜗牛 <1105066510@qq.com>
To: Tender Wang <tndrwang@gmail.com>
Cc: jian he <jian.universality@gmail.com>
Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Cc: aleksander <aleksander@timescale.com>
Subject: 回复： BUG #18568: BUG: Result wrong when do group by on partition table!
Date: Thu, 8 Aug 2024 19:21:05 +0800
Message-ID: <tencent_BAB00BBF45A08D941DF8290D9533941BB807@qq.com> (raw)
In-Reply-To: <CAHewXN=udOCEofnkRT8f-pCpKEVWqJ9Y2LwBe3JmxaJkaUOPiQ@mail.gmail.com>
References: <tencent_9D9103CDA420C07768349CC1DFF88465F90A@qq.com>
<CACJufxGeTcuRDM2OEq464bHmeuKiLz21GzFX+p7SDrMqi+9Pug@mail.gmail.com>
<CACJufxFX4Bw0xh-ex8cNakB1852d5x8kCwsFrkUcW4=3=qJvog@mail.gmail.com>
<CAHewXNnxrknh4ieTT78qOqHCMEt=ZBTi2BqJZPNYy8kkM70otg@mail.gmail.com>
<tencent_43E0973428CF0C6F3EC22CFC06807C533809@qq.com>
<CAHewXN=udOCEofnkRT8f-pCpKEVWqJ9Y2LwBe3JmxaJkaUOPiQ@mail.gmail.com>
Oh no!
Sorry, I made a mistake on patch.
v7 is new patch.
狂奔的蜗牛
1105066510@qq.com
------------------ 原始邮件 ------------------
发件人: "Tender Wang" <tndrwang@gmail.com>;
发送时间: 2024年8月8日(星期四) 下午5:46
收件人: "狂奔的蜗牛"<1105066510@qq.com>;
抄送: "jian he"<jian.universality@gmail.com>;"pgsql-bugs"<pgsql-bugs@lists.postgresql.org>;"aleksander"<aleksander@timescale.com>;
主题: Re: BUG #18568: BUG: Result wrong when do group by on partition table!
狂奔的蜗牛 <1105066510@qq.com> 于2024年8月8日周四 16:44写道：
I agree with advice of code style, and I have trim trailing whitespace in V6 patch.
The v6 patch still has code format issue, and the v6 looks almost same to v5.
git am v6-0001-fix-group_by_has_partkey-bug-and-add-regress-test.patch
Applying: fix-group_by_has_partkey-bug-and-add-regress-test
.git/rebase-apply/patch:31: trailing whitespace.
* Set collation in group by clause using COLLATE syntax,
.git/rebase-apply/patch:41: trailing whitespace.
}
--
Tender Wang
Attachments:
[application/octet-stream] v7-0001-fix-group_by_has_partkey-bug-and-add-regress-test.patch (8.8K, ../tencent_BAB00BBF45A08D941DF8290D9533941BB807@qq.com/3-v7-0001-fix-group_by_has_partkey-bug-and-add-regress-test.patch)
download | inline diff:
From b4fa71bef855976d65427010dbf71fdf872a609b Mon Sep 17 00:00:00 2001
From: hanweibo <hanweibo@highgo.com>
Date: Thu, 8 Aug 2024 19:17:18 +0800
Subject: [PATCH] fix-group_by_has_partkey-bug-and-add-regress-test
---
src/backend/optimizer/plan/planner.c | 25 +++-
.../regress/expected/partition_aggregate.out | 124 ++++++++++++++++++
src/test/regress/sql/partition_aggregate.sql | 40 ++++++
3 files changed, 186 insertions(+), 3 deletions(-)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.cindex 948afd9094..d04e622c24 100644--- a/src/backend/optimizer/plan/planner.c+++ b/src/backend/optimizer/plan/planner.c@@ -8025,13 +8025,32 @@ group_by_has_partkey(RelOptInfo *input_rel,
foreach(lc, partexprs)
{
+ ListCell *lg;
Expr *partexpr = lfirst(lc);
+ Oid partcoll = input_rel->part_scheme->partcollation[cnt];- if (list_member(groupexprs, partexpr))+ foreach (lg, groupexprs)
{
- found = true;- break;+ Expr *groupexpr = lfirst(lg);+ Oid groupcoll = exprCollation((Node *) groupexpr);++ /*+ * GROUP BY xx COLLATE xxx+ * Set collation in group by clause using COLLATE syntax,+ * AND the collation IS NOT equal to column's default collation.+ */+ if (IsA(groupexpr, RelabelType))+ groupexpr = ((RelabelType *) groupexpr)->arg;++ if (equal(groupexpr, partexpr) && partcoll == groupcoll)+ {+ found = true;+ break;+ }
}
++ if (found)+ break;
}
/*
diff --git a/src/test/regress/expected/partition_aggregate.out b/src/test/regress/expected/partition_aggregate.outindex 5f2c0cf578..38cc35f9f3 100644--- a/src/test/regress/expected/partition_aggregate.out+++ b/src/test/regress/expected/partition_aggregate.out@@ -1518,3 +1518,127 @@ SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) <
21 | 6000 | 6.0000000000000000 | 1000
(6 rows)
+-- Add collation check between groupkey and partkey.+-- If groupkey's collation is not equal to partkey's, we cannot use PARTITIONWISE_AGGREGATE_FULL.+SET enable_partitionwise_aggregate TO true;+SET enable_partitionwise_join TO true;+SET max_parallel_workers_per_gather TO 0;+SET enable_incremental_sort TO off;+CREATE COLLATION case_insensitive (+ provider = icu,+ locale = 'und-u-ks-level2',+ deterministic = false+);+CREATE TABLE pagg_tab3 (c text collate case_insensitive) PARTITION BY LIST(c collate "C");+CREATE TABLE pagg_tab3_p1 PARTITION OF pagg_tab3 FOR VALUES IN ('a', 'b', 'c', 'd');+CREATE TABLE pagg_tab3_p2 PARTITION OF pagg_tab3 FOR VALUES IN ('e', 'f', 'A');+CREATE TABLE pagg_tab3_p3 PARTITION OF pagg_tab3 FOR VALUES IN ('B', 'C', 'D', 'E');+INSERT INTO pagg_tab3 SELECT substr('abcdeABCDE', (i % 10) +1 , 1) FROM generate_series(0, 2999) i;+ANALYZE pagg_tab3;+EXPLAIN (COSTS OFF)+SELECT upper(c collate case_insensitive), count(c) FROM pagg_tab3 GROUP BY c collate case_insensitive ORDER BY 1;+ QUERY PLAN +--------------------------------------------------------------------+ Sort+ Sort Key: (upper(pagg_tab3.c)) COLLATE case_insensitive+ -> Finalize GroupAggregate+ Group Key: pagg_tab3.c+ -> Sort+ Sort Key: pagg_tab3.c COLLATE case_insensitive+ -> Append+ -> Partial HashAggregate+ Group Key: pagg_tab3.c+ -> Seq Scan on pagg_tab3_p2 pagg_tab3+ -> Partial HashAggregate+ Group Key: pagg_tab3_1.c+ -> Seq Scan on pagg_tab3_p3 pagg_tab3_1+ -> Partial HashAggregate+ Group Key: pagg_tab3_2.c+ -> Seq Scan on pagg_tab3_p1 pagg_tab3_2+(16 rows)++SELECT upper(c collate case_insensitive), count(c) FROM pagg_tab3 GROUP BY c collate case_insensitive ORDER BY 1;+ upper | count +-------+-------+ A | 600+ B | 600+ C | 600+ D | 600+ E | 600+(5 rows)++-- group key's collation not equal to column's, but equal to partkey's. We can also use PARTITIONWISE_AGGREGATE_FULL.+EXPLAIN (COSTS OFF)+SELECT c collate "C", count(c) FROM pagg_tab3 GROUP BY c collate "C" ORDER BY 1;+ QUERY PLAN +--------------------------------------------------------+ Sort+ Sort Key: ((pagg_tab3.c)::text) COLLATE "C"+ -> Append+ -> HashAggregate+ Group Key: (pagg_tab3.c)::text+ -> Seq Scan on pagg_tab3_p2 pagg_tab3+ -> HashAggregate+ Group Key: (pagg_tab3_1.c)::text+ -> Seq Scan on pagg_tab3_p3 pagg_tab3_1+ -> HashAggregate+ Group Key: (pagg_tab3_2.c)::text+ -> Seq Scan on pagg_tab3_p1 pagg_tab3_2+(12 rows)++SELECT c collate "C", count(c) FROM pagg_tab3 GROUP BY c collate "C" ORDER BY 1;+ c | count +---+-------+ A | 300+ B | 300+ C | 300+ D | 300+ E | 300+ a | 300+ b | 300+ c | 300+ d | 300+ e | 300+(10 rows)++-- pushdown having filter to sub table to improve performance+EXPLAIN (COSTS OFF)+SELECT c collate "C", count(c) FROM pagg_tab3 GROUP BY c collate "C" HAVING count(c) > 1 ORDER BY 1;+ QUERY PLAN +--------------------------------------------------------+ Sort+ Sort Key: ((pagg_tab3.c)::text) COLLATE "C"+ -> Append+ -> HashAggregate+ Group Key: (pagg_tab3.c)::text+ Filter: (count(pagg_tab3.c) > 1)+ -> Seq Scan on pagg_tab3_p2 pagg_tab3+ -> HashAggregate+ Group Key: (pagg_tab3_1.c)::text+ Filter: (count(pagg_tab3_1.c) > 1)+ -> Seq Scan on pagg_tab3_p3 pagg_tab3_1+ -> HashAggregate+ Group Key: (pagg_tab3_2.c)::text+ Filter: (count(pagg_tab3_2.c) > 1)+ -> Seq Scan on pagg_tab3_p1 pagg_tab3_2+(15 rows)++SELECT c collate "C", count(c) FROM pagg_tab3 GROUP BY c collate "C" HAVING count(c) > 1 ORDER BY 1;+ c | count +---+-------+ A | 300+ B | 300+ C | 300+ D | 300+ E | 300+ a | 300+ b | 300+ c | 300+ d | 300+ e | 300+(10 rows)++RESET enable_partitionwise_aggregate;+RESET enable_partitionwise_join;+RESET max_parallel_workers_per_gather;+RESET enable_incremental_sort;diff --git a/src/test/regress/sql/partition_aggregate.sql b/src/test/regress/sql/partition_aggregate.sqlindex ab070fee24..007d677198 100644--- a/src/test/regress/sql/partition_aggregate.sql+++ b/src/test/regress/sql/partition_aggregate.sql@@ -334,3 +334,43 @@ RESET parallel_setup_cost;
EXPLAIN (COSTS OFF)
SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3;
SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3;
+++-- Add collation check between groupkey and partkey.+-- If groupkey's collation is not equal to partkey's, we cannot use PARTITIONWISE_AGGREGATE_FULL.+SET enable_partitionwise_aggregate TO true;+SET enable_partitionwise_join TO true;+SET max_parallel_workers_per_gather TO 0;+SET enable_incremental_sort TO off;++CREATE COLLATION case_insensitive (+ provider = icu,+ locale = 'und-u-ks-level2',+ deterministic = false+);++CREATE TABLE pagg_tab3 (c text collate case_insensitive) PARTITION BY LIST(c collate "C");+CREATE TABLE pagg_tab3_p1 PARTITION OF pagg_tab3 FOR VALUES IN ('a', 'b', 'c', 'd');+CREATE TABLE pagg_tab3_p2 PARTITION OF pagg_tab3 FOR VALUES IN ('e', 'f', 'A');+CREATE TABLE pagg_tab3_p3 PARTITION OF pagg_tab3 FOR VALUES IN ('B', 'C', 'D', 'E');+INSERT INTO pagg_tab3 SELECT substr('abcdeABCDE', (i % 10) +1 , 1) FROM generate_series(0, 2999) i;+ANALYZE pagg_tab3;++EXPLAIN (COSTS OFF)+SELECT upper(c collate case_insensitive), count(c) FROM pagg_tab3 GROUP BY c collate case_insensitive ORDER BY 1;+SELECT upper(c collate case_insensitive), count(c) FROM pagg_tab3 GROUP BY c collate case_insensitive ORDER BY 1;++-- group key's collation not equal to column's, but equal to partkey's. We can also use PARTITIONWISE_AGGREGATE_FULL.+EXPLAIN (COSTS OFF)+SELECT c collate "C", count(c) FROM pagg_tab3 GROUP BY c collate "C" ORDER BY 1;+SELECT c collate "C", count(c) FROM pagg_tab3 GROUP BY c collate "C" ORDER BY 1;++-- pushdown having filter to sub table to improve performance+EXPLAIN (COSTS OFF)+SELECT c collate "C", count(c) FROM pagg_tab3 GROUP BY c collate "C" HAVING count(c) > 1 ORDER BY 1;+SELECT c collate "C", count(c) FROM pagg_tab3 GROUP BY c collate "C" HAVING count(c) > 1 ORDER BY 1;++RESET enable_partitionwise_aggregate;+RESET enable_partitionwise_join;+RESET max_parallel_workers_per_gather;+RESET enable_incremental_sort;--
2.25.1
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: 1105066510@qq.com, tndrwang@gmail.com, jian.universality@gmail.com, pgsql-bugs@lists.postgresql.org, aleksander@timescale.com
Subject: Re: =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?=
In-Reply-To: <tencent_BAB00BBF45A08D941DF8290D9533941BB807@qq.com>
* 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