pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feed=?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?=
7+ messages / 1 participants
[nested] [flat]
* =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?=
@ 2024-08-06 11:33 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-07 05:35 ` =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?= =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
1 sibling, 1 reply; 7+ messages in thread
From: 狂奔的蜗牛 @ 2024-08-06 11:33 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Partkey's collation stored in RelOptInfo->part_scheme, and I use it to fix the bug.
The attachment is my solution!
狂奔的蜗牛
1105066510@qq.com
------------------ 原始邮件 ------------------
发件人: "Tender Wang" <tndrwang@gmail.com>;
发送时间: 2024年8月6日(星期二) 下午4:42
收件人: "狂奔的蜗牛"<1105066510@qq.com>;"pgsql-bugs"<pgsql-bugs@lists.postgresql.org>;
主题: Re: BUG #18568: BUG: Result wrong when do group by on partition table!
PG Bug reporting form <noreply@postgresql.org> 于2024年8月6日周二 15:01写道：
The following bug has been logged on the website:
Bug reference: 18568
Logged by: Webbo Han
Email address: 1105066510@qq.com
PostgreSQL version: 16.3
Operating system: centos 7.6
Description:
First, we create one case-insensitive collation use ICU:
```sql
CREATE COLLATION case_insensitive (
provider = icu,
locale = 'und-u-ks-level2',
deterministic = false
);
```
Then, we create the partition table, meanwhile we set the collation of
column c to `case_insensitive`,
and set partkey's collation to 'C'.
```sql
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 TABLE pagg_tab (c text collate case_insensitive) PARTITION BY LIST(c
collate "C");
CREATE TABLE pagg_tab_p1 PARTITION OF pagg_tab FOR VALUES IN ('a', 'b',
'c', 'd');
CREATE TABLE pagg_tab_p2 PARTITION OF pagg_tab FOR VALUES IN ('e', 'f',
'A');
CREATE TABLE pagg_tab_p3 PARTITION OF pagg_tab FOR VALUES IN ('B', 'C',
'D', 'E');
INSERT INTO pagg_tab SELECT substr('abcdeABCDE', (i % 10) +1 , 1) FROM
generate_series(0, 2999) i;
ANALYZE pagg_tab;
```
We do group by on the table pagg_tab use `case_insensitive` collation, we
hope group key is case-insensitive.
but we find the execution result is not what we expected.
```shell
postgres=# SELECT c collate case_insensitive, count(c) FROM pagg_tab GROUP
BY c collate case_insensitive;
c | count
---+-------
A | 300
e | 300
E | 300
D | 300
C | 300
B | 300
d | 300
c | 300
b | 300
a | 300
(10 rows)
```
The reason is the function group_by_has_partkey() do not check partkey's
collation, that lead to explain error.
```shell
postgres=# EXPLAIN SELECT c collate case_insensitive, count(c) FROM
pagg_tab GROUP BY c collate case_insensitive ;
QUERY PLAN
--------------------------------------------------------------------------------------
Append (cost=12.00..60.15 rows=10 width=10)
-> HashAggregate (cost=12.00..12.02 rows=2 width=10)
Group Key: pagg_tab.c
-> Seq Scan on pagg_tab_p2 pagg_tab (cost=0.00..9.00 rows=600
width=2)
-> HashAggregate (cost=24.00..24.04 rows=4 width=10)
Group Key: pagg_tab_1.c
-> Seq Scan on pagg_tab_p3 pagg_tab_1 (cost=0.00..18.00
rows=1200 width=2)
-> HashAggregate (cost=24.00..24.04 rows=4 width=10)
Group Key: pagg_tab_2.c
-> Seq Scan on pagg_tab_p1 pagg_tab_2 (cost=0.00..18.00
rows=1200 width=2)
(10 rows)
```
So, group_by_has_partkey() need to verify if the partkey's collation matches
the groupkey,
meanwhile, if groupkey is RelabelType node and it's collation equal to
partkey's, it should
also set variable `found` to true.
Yeah, I can reproduce $subject on HEAD.But I found this when debug into group_by_has_partkey(), as below:
call nodeToString(groupexprs):
VAR : varcollid 16384
call nodeToString(partexpr):
VAR: varcollid 16384
So the collid of partkey and groupexpr is same, so add check here may not fix this issue.
I continue to find out why the collation id of partkey is 16384(e.g. case_insensitive). The partkey expr info is
set in set_baserel_partition_key_exprs(), which it uses partkey->parttypcoll[cnt] value not partkey->partcollation value.
And partkey->parttypcoll[cnt] is assigned from pg_attribute , which is the column c meta data.
Should we use partkey->partcollation value? I try to fix that in the attached patch. I add your case in the test, and I don't find
failed regress.
--
Tender Wang
Attachments:
[application/octet-stream] 0001-fix-group_by_has_partkey-bug.patch (1.6K, ../../tencent_30047C85C6C819CECD4318951F101077CA06@qq.com/3-0001-fix-group_by_has_partkey-bug.patch)
download | inline diff:
From 01df112261a8cd2e6e7b6befd163ebdee5d01304 Mon Sep 17 00:00:00 2001
From: hanweibo <hanweibo@highgo.com>
Date: Tue, 6 Aug 2024 19:26:09 +0800
Subject: [PATCH] fix group_by_has_partkey bug
---
src/backend/optimizer/plan/planner.c | 35 +++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 948afd9094..8cdece261f 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -8025,13 +8025,42 @@ 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);
+
+ /*
+ * 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 (nodeTag(groupexpr) == T_RelabelType)
+ {
+ RelabelType *rt = (RelabelType *) groupexpr;
+
+ if (equal(rt->arg, partexpr) && rt->resultcollid == partcoll)
+ {
+ found = true;
+ break;
+ }
+ }
+ /*
+ * Not set collation in group by clause, or the collation
+ * is the same as column's.
+ */
+ else if (equal(groupexpr, partexpr) && exprCollation((Node *) groupexpr) == partcoll)
+ {
+ found = true;
+ break;
+ }
}
+
+ if (found)
+ break;
}
/*
--
2.25.1
^ permalink raw reply [nested|flat] 7+ messages in thread
* =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?=
2024-08-06 11:33 ` =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?= =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
@ 2024-08-07 05:35 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-07 08:06 ` =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?= =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
0 siblings, 1 reply; 7+ messages in thread
From: 狂奔的蜗牛 @ 2024-08-07 05:35 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
this case will enter `if (IsA(groupexpr, RelabelType))` branch.
We set "C" as groupkey's collation, and it's not equal to column's.
In transformGroupClause(), syntax `COLLATE xxx` will create CollateExpr node, and CollateExpr's arg is Var node.
And then, planner will call eval_const_expressions_mutator() to transform CollateExpr to RelableType if CollateExpr->collOid not equal to Collate->arg's collation.
About V3 patch, PartCollMatchesExprColl() may be not suitable, because collation of partkey must be equal to groupkey's, even though they are all InvalidOid.
```sql
CREATE COLLATION case_insensitive (
provider = icu,
locale = 'und-u-ks-level2',
deterministic = false
);
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;
drop table pagg_tab;
CREATE TABLE pagg_tab (c text collate case_insensitive) PARTITION BY LIST(c collate "C");
CREATE TABLE pagg_tab_p1 PARTITION OF pagg_tab FOR VALUES IN ('a', 'b', 'c', 'd');
CREATE TABLE pagg_tab_p2 PARTITION OF pagg_tab FOR VALUES IN ('e', 'f', 'A');
CREATE TABLE pagg_tab_p3 PARTITION OF pagg_tab FOR VALUES IN ('B', 'C', 'D', 'E');
INSERT INTO pagg_tab SELECT substr('abcdeABCDE', (i % 10) +1 , 1) FROM generate_series(0, 2999) i;
ANALYZE pagg_tab;
EXPLAIN SELECT c collate "C", count(c) FROM pagg_tab GROUP BY c collate "C" ;
SELECT c collate "C", count(c) FROM pagg_tab GROUP BY c collate "C";
```
狂奔的蜗牛
1105066510@qq.com
------------------ 原始邮件 ------------------
发件人: "Tender Wang" <tndrwang@gmail.com>;
发送时间: 2024年8月7日(星期三) 中午12:01
收件人: "狂奔的蜗牛"<1105066510@qq.com>;
抄送: "pgsql-bugs"<pgsql-bugs@lists.postgresql.org>;
主题: Re: BUG #18568: BUG: Result wrong when do group by on partition table!
I think what I have done in v2 is not the right way. Because partition prune logic first checks whether it is equalor not, then it will check the collation match or not. So I should not change the set_baserel_partition_key_exprs() logic.
I look through your patch and make some changes.
1.
git am your patch, report warnings, some code format issue.
2.
I removed this branch: if (IsA(groupexpr, RelabelType)).
Because in your added test case, it didn't enter this branch. I didn't figure out what kind of group by clause is RelableType.
You can provide a test case based on the v3 patch.
3. Tweek a little about the test case.
By the way, your last two emails only sent to me, please cc the pgsql-bugs.
狂奔的蜗牛 <1105066510@qq.com> 于2024年8月6日周二 19:33写道：
Partkey's collation stored in RelOptInfo->part_scheme, and I use it to fix the bug.
The attachment is my solution!
狂奔的蜗牛
1105066510@qq.com
------------------ 原始邮件 ------------------
发件人: "Tender Wang" <tndrwang@gmail.com>;
发送时间: 2024年8月6日(星期二) 下午4:42
收件人: "狂奔的蜗牛"<1105066510@qq.com>;"pgsql-bugs"<pgsql-bugs@lists.postgresql.org>;
主题: Re: BUG #18568: BUG: Result wrong when do group by on partition table!
PG Bug reporting form <noreply@postgresql.org> 于2024年8月6日周二 15:01写道：
The following bug has been logged on the website:
Bug reference: 18568
Logged by: Webbo Han
Email address: 1105066510@qq.com
PostgreSQL version: 16.3
Operating system: centos 7.6
Description:
First, we create one case-insensitive collation use ICU:
```sql
CREATE COLLATION case_insensitive (
provider = icu,
locale = 'und-u-ks-level2',
deterministic = false
);
```
Then, we create the partition table, meanwhile we set the collation of
column c to `case_insensitive`,
and set partkey's collation to 'C'.
```sql
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 TABLE pagg_tab (c text collate case_insensitive) PARTITION BY LIST(c
collate "C");
CREATE TABLE pagg_tab_p1 PARTITION OF pagg_tab FOR VALUES IN ('a', 'b',
'c', 'd');
CREATE TABLE pagg_tab_p2 PARTITION OF pagg_tab FOR VALUES IN ('e', 'f',
'A');
CREATE TABLE pagg_tab_p3 PARTITION OF pagg_tab FOR VALUES IN ('B', 'C',
'D', 'E');
INSERT INTO pagg_tab SELECT substr('abcdeABCDE', (i % 10) +1 , 1) FROM
generate_series(0, 2999) i;
ANALYZE pagg_tab;
```
We do group by on the table pagg_tab use `case_insensitive` collation, we
hope group key is case-insensitive.
but we find the execution result is not what we expected.
```shell
postgres=# SELECT c collate case_insensitive, count(c) FROM pagg_tab GROUP
BY c collate case_insensitive;
c | count
---+-------
A | 300
e | 300
E | 300
D | 300
C | 300
B | 300
d | 300
c | 300
b | 300
a | 300
(10 rows)
```
The reason is the function group_by_has_partkey() do not check partkey's
collation, that lead to explain error.
```shell
postgres=# EXPLAIN SELECT c collate case_insensitive, count(c) FROM
pagg_tab GROUP BY c collate case_insensitive ;
QUERY PLAN
--------------------------------------------------------------------------------------
Append (cost=12.00..60.15 rows=10 width=10)
-> HashAggregate (cost=12.00..12.02 rows=2 width=10)
Group Key: pagg_tab.c
-> Seq Scan on pagg_tab_p2 pagg_tab (cost=0.00..9.00 rows=600
width=2)
-> HashAggregate (cost=24.00..24.04 rows=4 width=10)
Group Key: pagg_tab_1.c
-> Seq Scan on pagg_tab_p3 pagg_tab_1 (cost=0.00..18.00
rows=1200 width=2)
-> HashAggregate (cost=24.00..24.04 rows=4 width=10)
Group Key: pagg_tab_2.c
-> Seq Scan on pagg_tab_p1 pagg_tab_2 (cost=0.00..18.00
rows=1200 width=2)
(10 rows)
```
So, group_by_has_partkey() need to verify if the partkey's collation matches
the groupkey,
meanwhile, if groupkey is RelabelType node and it's collation equal to
partkey's, it should
also set variable `found` to true.
Yeah, I can reproduce $subject on HEAD.But I found this when debug into group_by_has_partkey(), as below:
call nodeToString(groupexprs):
VAR : varcollid 16384
call nodeToString(partexpr):
VAR: varcollid 16384
So the collid of partkey and groupexpr is same, so add check here may not fix this issue.
I continue to find out why the collation id of partkey is 16384(e.g. case_insensitive). The partkey expr info is
set in set_baserel_partition_key_exprs(), which it uses partkey->parttypcoll[cnt] value not partkey->partcollation value.
And partkey->parttypcoll[cnt] is assigned from pg_attribute , which is the column c meta data.
Should we use partkey->partcollation value? I try to fix that in the attached patch. I add your case in the test, and I don't find
failed regress.
--
Tender Wang
--
Tender Wang
^ permalink raw reply [nested|flat] 7+ messages in thread
* =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?=
2024-08-06 11:33 ` =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?= =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-07 05:35 ` =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?= =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
@ 2024-08-07 08:06 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
0 siblings, 0 replies; 7+ messages in thread
From: 狂奔的蜗牛 @ 2024-08-07 08:06 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>; aleksander <aleksander@timescale.com>
PartCollMatchesExprColl() is not strict enough.
If partcoll == InvalidOid and groupcoll != InvalidOid, PartCollMatchesExprColl() return true always.
Just determine whether groupcoll is equal to partcoll, like this "partcoll == groupcoll".
We cannot delete "if (IsA(groupexpr, RelableTyple) " branch,
becasuse if groupexpr is RelabelType and partcoll equal to groupcoll, the "equal(groupexpr, partexpr) && PartKeyCollMatchesExprColl(partcoll, groupexpr_coll)" condition return false!!!
This is not what we expect.
We could rename "groupexpr_coll" to groupcoll, it looks more elegant.
狂奔的蜗牛
1105066510@qq.com
------------------ 原始邮件 ------------------
发件人: "Tender Wang" <tndrwang@gmail.com>;
发送时间: 2024年8月7日(星期三) 下午2:57
收件人: "狂奔的蜗牛"<1105066510@qq.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月7日周三 13:35写道：
this case will enter `if (IsA(groupexpr, RelabelType))` branch.
We set "C" as groupkey's collation, and it's not equal to column's.
In transformGroupClause(), syntax `COLLATE xxx` will create CollateExpr node, and CollateExpr's arg is Var node.
And then, planner will call eval_const_expressions_mutator() to transform CollateExpr to RelableType if CollateExpr->collOid not equal to Collate->arg's collation.
Yech. Thanks for the explanation.
Because exprCollation() return the resultcollid if the nodetag is Relabletype, the expr() && PartKeyCollMatchesExprColl() can handle this situation.
So we don't need "if (IsA(groupexpr, RelableTyple) " this branch.
About V3 patch, PartCollMatchesExprColl() may be not suitable, because collation of partkey must be equal to groupkey's, even though they are all InvalidOid.
I'm not sure about what you said. I uesd it because partition prune do this way. I keep it temporarily.
The test case in v3 patch only has EXPLAIN statement. I add SELECT statement and make the result more stable according to Aleksander advices.
--
Tender Wang
^ permalink raw reply [nested|flat] 7+ messages in thread
* =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?=
@ 2024-08-06 11:45 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
1 sibling, 0 replies; 7+ messages in thread
From: 狂奔的蜗牛 @ 2024-08-06 11:45 UTC (permalink / raw)
To: Aleksander Alekseev <aleksander@timescale.com>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>; +Cc: Tender Wang <tndrwang@gmail.com>
Hi,
I am working to compatible with Oracle in PG,
in oracle, the result used case-insensitive collation is not unique.
There are two oracle's test case:
first case:
```sql
CREATE TABLE group_by_ci_test_1 (c varchar2(20));
INSERT INTO group_by_ci_test_1 VALUES ('a');
INSERT INTO group_by_ci_test_1 VALUES ('a');
INSERT INTO group_by_ci_test_1 VALUES ('A');
SELECT c collate binary_ci, count(c) FROM group_by_ci_test_1 GROUP BY c collate binary_ci;
```
first result
```shell
CCOLLATEBINARY_CI COUNT(C)
-------------------- ----------
a 3
```
second case:
```sql
CREATE TABLE group_by_ci_test_2 (c varchar2(20));
INSERT INTO group_by_ci_test_2 VALUES ('A');
INSERT INTO group_by_ci_test_2 VALUES ('a');
INSERT INTO group_by_ci_test_2 VALUES ('a');
SELECT c collate binary_ci, count(c) FROM group_by_ci_test_2 GROUP BY c collate binary_ci;
```
second result:
```shell
CCOLLATEBINARY_CI COUNT(C)
-------------------- ----------
A 6
```
狂奔的蜗牛
1105066510@qq.com
------------------ 原始邮件 ------------------
发件人: "Aleksander Alekseev" <aleksander@timescale.com>;
发送时间: 2024年8月6日(星期二) 下午5:09
收件人: "pgsql-bugs"<pgsql-bugs@lists.postgresql.org>;
抄送: "Tender Wang"<tndrwang@gmail.com>;"狂奔的蜗牛"<1105066510@qq.com>;
主题: Re: BUG #18568: BUG: Result wrong when do group by on partition table!
Hi,
> [...]
> I continue to find out why the collation id of partkey is 16384(e.g. case_insensitive). The partkey expr info is
> set in set_baserel_partition_key_exprs(), which it uses partkey->parttypcoll[cnt] value not partkey->partcollation value.
>
> And partkey->parttypcoll[cnt] is assigned from pg_attribute , which is the column c meta data.
> Should we use partkey->partcollation value? I try to fix that in the attached patch. I add your case in the test, and I don't find
> failed regress.
```
+SELECT c collate case_insensitive, count(c) FROM
+pagg_tab_col GROUP BY c collate case_insensitive;
+ c | count
+---+-------
+ e | 600
+ D | 600
+ C | 600
+ B | 600
+ A | 600
+(5 rows)
```
Shouldn't we use UPPER(c) and ORDER BY in the test case to make the
results deterministic?
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 7+ messages in thread
* =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?=
@ 2024-08-07 10:38 =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-08 08:44 ` =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?= =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
0 siblings, 1 reply; 7+ messages in thread
From: 狂奔的蜗牛 @ 2024-08-07 10:38 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>; aleksander <aleksander@timescale.com>
I readd RelabelType to branch in v5 patch, and add case to cover the code.
In new adding case: `SELECT c collate "C", count(c) FROM pagg_tab3 GROUP BY c collate "C" HAVING count(c) > 1 ORDER BY 1;`
According to original logic, group_by_has_partkey() will return false, it will lead to cannot do having filter in sub table, the result is correct but the performance is poor.
Our logic will get better performance.
About InvalidOid collation, non-character type's collation is 0(InvalidOid) always.
狂奔的蜗牛
1105066510@qq.com
------------------ 原始邮件 ------------------
发件人: "Tender Wang" <tndrwang@gmail.com>;
发送时间: 2024年8月7日(星期三) 下午5:23
收件人: "狂奔的蜗牛"<1105066510@qq.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月7日周三 16:06写道：
PartCollMatchesExprColl() is not strict enough.
If partcoll == InvalidOid and groupcoll != InvalidOid, PartCollMatchesExprColl() return true always.
Just determine whether groupcoll is equal to partcoll, like this "partcoll == groupcoll".
I understand what you said. Actually, I keep it just curious when partcoll is InvalidOid. Why it works for partition prune.
Is it not same between check partkey is equal to groupexpr and check partkey is equal to qualclause?
We cannot delete "if (IsA(groupexpr, RelableTyple) " branch,
becasuse if groupexpr is RelabelType and partcoll equal to groupcoll, the "equal(groupexpr, partexpr) && PartKeyCollMatchesExprColl(partcoll, groupexpr_coll)" condition return false!!!
This is not what we expect.
" if groupexpr is RelabelType and partcoll equal to groupcoll ", according to original logic, will return false in this situation.
Now you think we can support above situation. Am I understand correctly?
We're better to add more test case to cover the code if we're going to support this. The test cases now seem not going
into the RelableTyple branch.
We could rename "groupexpr_coll" to groupcoll, it looks more elegant.
No objection. You can continue to support RelableType situation and add more test cases based on V4.
--
Tender Wang
Attachments:
[application/octet-stream] v5-0001-fix-group_by_has_partkey-bug-and-add-regress-test.patch (8.8K, ../../tencent_9D9103CDA420C07768349CC1DFF88465F90A@qq.com/3-v5-0001-fix-group_by_has_partkey-bug-and-add-regress-test.patch)
download | inline diff:
From 8ea49b45f9bcd08a08021c70c8adbd2d6c475f3f Mon Sep 17 00:00:00 2001
From: hanweibo <hanweibo@highgo.com>
Date: Wed, 7 Aug 2024 18:35:14 +0800
Subject: [PATCH] fix-group_by_has_partkey-bug-and-add-regress-test
---
src/backend/optimizer/plan/planner.c | 28 +++-
.../regress/expected/partition_aggregate.out | 124 ++++++++++++++++++
src/test/regress/sql/partition_aggregate.sql | 41 ++++++
3 files changed, 190 insertions(+), 3 deletions(-)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 948afd9094..df5ebb8943 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -8025,13 +8025,35 @@ 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);
+
+ if (partcoll == groupcoll)
+ {
+ /*
+ * 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))
+ {
+ found = true;
+ break;
+ }
+ }
}
+
+ if (found)
+ break;
}
/*
diff --git a/src/test/regress/expected/partition_aggregate.out b/src/test/regress/expected/partition_aggregate.out
index 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.sql
index ab070fee24..f4c2719d3a 100644
--- a/src/test/regress/sql/partition_aggregate.sql
+++ b/src/test/regress/sql/partition_aggregate.sql
@@ -334,3 +334,44 @@ 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
^ permalink raw reply [nested|flat] 7+ messages in thread
* =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?=
2024-08-07 10:38 =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?= =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
@ 2024-08-08 08:44 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-08 11:21 ` =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?= =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
0 siblings, 1 reply; 7+ messages in thread
From: 狂奔的蜗牛 @ 2024-08-08 08:44 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; jian he <jian.universality@gmail.com>; +Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>; aleksander <aleksander@timescale.com>
I agree with advice of code style, and I have trim trailing whitespace in V6 patch.
About test case added in file partition_aggregate.sql, for we create case_insentitive collation based on ICU, which regress sql file we should write is better ?
If write into collate.icu.utf8.sql, it seems not suitable.
狂奔的蜗牛
1105066510@qq.com
------------------ 原始邮件 ------------------
发件人: "Tender Wang" <tndrwang@gmail.com>;
发送时间: 2024年8月8日(星期四) 下午4:20
收件人: "jian he"<jian.universality@gmail.com>;
抄送: "狂奔的蜗牛"<1105066510@qq.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!
jian he <jian.universality@gmail.com> 于2024年8月8日周四 15:51写道：
On Wed, Aug 7, 2024 at 11:04�6�2PM jian he <jian.universality@gmail.com> wrote:
>
> On Wed, Aug 7, 2024 at 9:08�6�2PM 狂奔的蜗牛 <1105066510@qq.com> wrote:
> >
> > I readd RelabelType to branch in v5 patch, and add case to cover the code.
> >
I did some minor adjustments in group_by_has_partkey, also refactored
the comment.
The logic in patch I think no problem. But I don't like the code style.
if (IsA(groupexpr, RelabelType))
groupexpr = ((RelabelType *) groupexpr)->arg;
if (expr(groupexpr, partexr) && partcoll == groupcoll)
{
...
}
This looks more compact.
By the way, I think we'd better to include test cases together in your patch, then others can continue
to work based on your patch not need to go back to add the test cases.
--
Tender Wang
Attachments:
[application/octet-stream] v6-0001-fix-group_by_has_partkey-bug-and-add-regress-test.patch (8.8K, ../../tencent_43E0973428CF0C6F3EC22CFC06807C533809@qq.com/3-v6-0001-fix-group_by_has_partkey-bug-and-add-regress-test.patch)
download | inline diff:
From 8ea49b45f9bcd08a08021c70c8adbd2d6c475f3f Mon Sep 17 00:00:00 2001
From: hanweibo <hanweibo@highgo.com>
Date: Wed, 7 Aug 2024 18:35:14 +0800
Subject: [PATCH] fix-group_by_has_partkey-bug-and-add-regress-test
---
src/backend/optimizer/plan/planner.c | 28 +++-
.../regress/expected/partition_aggregate.out | 124 ++++++++++++++++++
src/test/regress/sql/partition_aggregate.sql | 41 ++++++
3 files changed, 190 insertions(+), 3 deletions(-)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 948afd9094..df5ebb8943 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -8025,13 +8025,35 @@ 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);
+
+ if (partcoll == groupcoll)
+ {
+ /*
+ * 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))
+ {
+ found = true;
+ break;
+ }
+ }
}
+
+ if (found)
+ break;
}
/*
diff --git a/src/test/regress/expected/partition_aggregate.out b/src/test/regress/expected/partition_aggregate.out
index 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.sql
index ab070fee24..f4c2719d3a 100644
--- a/src/test/regress/sql/partition_aggregate.sql
+++ b/src/test/regress/sql/partition_aggregate.sql
@@ -334,3 +334,44 @@ 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
^ permalink raw reply [nested|flat] 7+ messages in thread
* =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?=
2024-08-07 10:38 =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?= =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-08 08:44 ` =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?= =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
@ 2024-08-08 11:21 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
0 siblings, 0 replies; 7+ messages in thread
From: 狂奔的蜗牛 @ 2024-08-08 11:21 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: jian he <jian.universality@gmail.com>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>; aleksander <aleksander@timescale.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.c
index 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.out
index 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.sql
index 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
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2024-08-08 11:21 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-08-06 11:33 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-07 05:35 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-07 08:06 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-06 11:45 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-07 10:38 =?gb18030?B?u9i4tKO6IEJVRyAjMTg1Njg6IEJVRzogUmVzdWx0?= =?gb18030?B?IHdyb25nIHdoZW4gZG8gZ3JvdXAgYnkgb24gcGFy?= =?gb18030?B?dGl0aW9uIHRhYmxlIQ==?= =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-08 08:44 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
2024-08-08 11:21 ` =?gb18030?B?v/GxvLXEzs/Fow==?= <1105066510@qq.com>
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