BUG #18568: BUG: Result wrong when do group by on partition table!

Started by PG Bug reporting formover 1 year ago27 messagesbugs
Jump to latest
#1PG Bug reporting form
noreply@postgresql.org

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.

#2Tender Wang
tndrwang@gmail.com
In reply to: PG Bug reporting form (#1)
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:

v1-0001-Fix-collation-different-between-columan-collation.patchapplication/octet-stream; name=v1-0001-Fix-collation-different-between-columan-collation.patchDownload+54-2
#3Aleksander Alekseev
aleksander@timescale.com
In reply to: Tender Wang (#2)
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

#4Tender Wang
tndrwang@gmail.com
In reply to: Tender Wang (#2)
Re: BUG #18568: BUG: Result wrong when do group by on partition table!

Tender Wang <tndrwang@gmail.com> 于2024年8月6日周二 16:42写道:

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.

The plan seems not what we want, because I forgot to
set enable_partitionwise_aggregate to true.

--
Tender Wang

Attachments:

v2-0001-Fix-collation-different-between-columan-collation.patchapplication/octet-stream; name=v2-0001-Fix-collation-different-between-columan-collation.patchDownload+75-2
#5狂奔的蜗牛
1105066510@qq.com
In reply to: Tender Wang (#2)
回复: BUG #18568: BUG: Result wrong when do group by on partition table!

Partkey's collation&nbsp;stored in&nbsp;RelOptInfo-&gt;part_scheme, and I use it to fix the bug.
The attachment is my solution!

狂奔的蜗牛
1105066510@qq.com

&nbsp;

------------------&nbsp;原始邮件&nbsp;------------------
发件人: "Tender Wang" <tndrwang@gmail.com&gt;;
发送时间:&nbsp;2024年8月6日(星期二) 下午4:42
收件人:&nbsp;"狂奔的蜗牛"<1105066510@qq.com&gt;;"pgsql-bugs"<pgsql-bugs@lists.postgresql.org&gt;;

主题:&nbsp;Re: BUG #18568: BUG: Result wrong when do group by on partition table!

PG Bug reporting form <noreply@postgresql.org&gt; 于2024年8月6日周二 15:01写道:

The following bug has been logged on the website:

Bug reference:&nbsp; &nbsp; &nbsp; 18568
Logged by:&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; Webbo Han
Email address:&nbsp; &nbsp; &nbsp; 1105066510@qq.com
PostgreSQL version: 16.3
Operating system:&nbsp; &nbsp;centos 7.6
Description:&nbsp; &nbsp; &nbsp; &nbsp;

First, we create one case-insensitive collation use ICU:
```sql
&nbsp; &nbsp; &nbsp; &nbsp; CREATE COLLATION case_insensitive (
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; provider = icu,
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; locale = 'und-u-ks-level2',
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; deterministic = false
&nbsp; &nbsp; &nbsp; &nbsp; );
```

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
&nbsp; &nbsp; &nbsp; &nbsp; SET enable_partitionwise_aggregate TO true;
&nbsp; &nbsp; &nbsp; &nbsp; SET enable_partitionwise_join TO true;
&nbsp; &nbsp; &nbsp; &nbsp; SET max_parallel_workers_per_gather TO 0;
&nbsp; &nbsp; &nbsp; &nbsp; SET enable_incremental_sort TO off;
&nbsp; &nbsp; &nbsp; &nbsp; CREATE TABLE pagg_tab (c text collate case_insensitive) PARTITION BY LIST(c
collate "C");
&nbsp; &nbsp; &nbsp; &nbsp; CREATE TABLE pagg_tab_p1 PARTITION OF pagg_tab FOR VALUES IN ('a', 'b',
'c', 'd');
&nbsp; &nbsp; &nbsp; &nbsp; CREATE TABLE pagg_tab_p2 PARTITION OF pagg_tab FOR VALUES IN ('e', 'f',
'A');
&nbsp; &nbsp; &nbsp; &nbsp; CREATE TABLE pagg_tab_p3 PARTITION OF pagg_tab FOR VALUES IN ('B', 'C',
'D', 'E');
&nbsp; &nbsp; &nbsp; &nbsp; INSERT INTO pagg_tab SELECT substr('abcdeABCDE', (i % 10) +1 , 1) FROM
generate_series(0, 2999) i;
&nbsp; &nbsp; &nbsp; &nbsp; 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
&nbsp; &nbsp; &nbsp; &nbsp; postgres=# SELECT c collate case_insensitive, count(c) FROM pagg_tab GROUP
BY c collate case_insensitive;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;c | count
&nbsp; &nbsp; &nbsp; &nbsp; ---+-------
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;A |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;e |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;E |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;D |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;C |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;B |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;d |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;c |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;b |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;a |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; (10 rows)
```

The reason is the function group_by_has_partkey() do not check partkey's
collation, that lead to explain error.
```shell
&nbsp; &nbsp; &nbsp; &nbsp; postgres=# EXPLAIN SELECT c collate case_insensitive, count(c) FROM
pagg_tab GROUP BY c collate case_insensitive ;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; QUERY PLAN
&nbsp; &nbsp; &nbsp; &nbsp; --------------------------------------------------------------------------------------
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;Append&nbsp; (cost=12.00..60.15 rows=10 width=10)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; HashAggregate&nbsp; (cost=12.00..12.02 rows=2 width=10)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;Group Key: pagg_tab.c
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; Seq Scan on pagg_tab_p2 pagg_tab&nbsp; (cost=0.00..9.00 rows=600
width=2)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; HashAggregate&nbsp; (cost=24.00..24.04 rows=4 width=10)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;Group Key: pagg_tab_1.c
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; Seq Scan on pagg_tab_p3 pagg_tab_1&nbsp; (cost=0.00..18.00
rows=1200 width=2)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; HashAggregate&nbsp; (cost=24.00..24.04 rows=4 width=10)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;Group Key: pagg_tab_2.c
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; Seq Scan on pagg_tab_p1 pagg_tab_2&nbsp; (cost=0.00..18.00
rows=1200 width=2)
&nbsp; &nbsp; &nbsp; &nbsp; (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&nbsp; 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-&gt;parttypcoll[cnt] value not&nbsp; partkey-&gt;partcollation value.

And partkey-&gt;parttypcoll[cnt] is assigned from pg_attribute , which is the column c meta data.&nbsp;
Should we use partkey-&gt;partcollation value?&nbsp; 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:

0001-fix-group_by_has_partkey-bug.patchapplication/octet-stream; charset=gb18030; name=0001-fix-group_by_has_partkey-bug.patchDownload+32-4
#6狂奔的蜗牛
1105066510@qq.com
In reply to: Aleksander Alekseev (#3)
回复: BUG #18568: BUG: Result wrong when do group by on partition table!

Hi,
I am working to compatible with Oracle in PG,
in oracle, the result used case-insensitive collation is not&nbsp;unique.
There are two oracle's test case:

first case:
```sql
&nbsp; &nbsp; CREATE TABLE group_by_ci_test_1 (c varchar2(20));
&nbsp; &nbsp; INSERT INTO group_by_ci_test_1 VALUES ('a');
&nbsp; &nbsp; INSERT INTO group_by_ci_test_1 VALUES ('a');
&nbsp; &nbsp; INSERT INTO group_by_ci_test_1 VALUES ('A');
&nbsp; &nbsp; SELECT c collate binary_ci, count(c) FROM group_by_ci_test_1 GROUP BY c collate binary_ci;
```
first result
```shell
&nbsp; &nbsp; CCOLLATEBINARY_CI&nbsp; &nbsp; &nbsp; COUNT(C)
&nbsp; &nbsp; -------------------- ----------
&nbsp; &nbsp; a&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;3

```

second case:
```sql
&nbsp; &nbsp; CREATE TABLE group_by_ci_test_2 (c varchar2(20));
&nbsp; &nbsp; INSERT INTO group_by_ci_test_2 VALUES ('A');
&nbsp; &nbsp; INSERT INTO group_by_ci_test_2 VALUES ('a');
&nbsp; &nbsp; INSERT INTO group_by_ci_test_2 VALUES ('a');
&nbsp; &nbsp; SELECT c collate binary_ci, count(c) FROM group_by_ci_test_2 GROUP BY c collate binary_ci;

```
second result:
```shell
&nbsp; &nbsp; CCOLLATEBINARY_CI&nbsp; &nbsp; &nbsp; COUNT(C)
&nbsp; &nbsp; -------------------- ----------
&nbsp; &nbsp; A&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;6

```

狂奔的蜗牛
1105066510@qq.com

&nbsp;

------------------&nbsp;原始邮件&nbsp;------------------
发件人: "Aleksander Alekseev" <aleksander@timescale.com&gt;;
发送时间:&nbsp;2024年8月6日(星期二) 下午5:09
收件人:&nbsp;"pgsql-bugs"<pgsql-bugs@lists.postgresql.org&gt;;
抄送:&nbsp;"Tender Wang"<tndrwang@gmail.com&gt;;"狂奔的蜗牛"<1105066510@qq.com&gt;;
主题:&nbsp;Re: BUG #18568: BUG: Result wrong when do group by on partition table!

Hi,

&gt; [...]
&gt; I continue to find out why the collation id of partkey is 16384(e.g. case_insensitive). The partkey expr info is
&gt; set in set_baserel_partition_key_exprs(), which it uses partkey-&gt;parttypcoll[cnt] value not&nbsp; partkey-&gt;partcollation value.
&gt;
&gt; And partkey-&gt;parttypcoll[cnt] is assigned from pg_attribute , which is the column c meta data.
&gt; Should we use partkey-&gt;partcollation value?&nbsp; I try to fix that in the attached patch. I add your case in the test, and I don't find
&gt; failed regress.

```
+SELECT c collate case_insensitive, count(c) FROM
+pagg_tab_col GROUP BY c collate case_insensitive;
+ c | count
+---+-------
+ e |&nbsp;&nbsp; 600
+ D |&nbsp;&nbsp; 600
+ C |&nbsp;&nbsp; 600
+ B |&nbsp;&nbsp; 600
+ A |&nbsp;&nbsp; 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

#7Tender Wang
tndrwang@gmail.com
In reply to: PG Bug reporting form (#1)
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");

I think above create table again. Should we allow this CREATE TABLE? The
partition key
definition are not same with column definiton. Is it better to report
error for users?

--
Tender Wang

#8Tender Wang
tndrwang@gmail.com
In reply to: 狂奔的蜗牛 (#5)
Re: BUG #18568: BUG: Result wrong when do group by on partition table!

狂奔的蜗牛 <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!

I look through your patch. Some code is not native for PG. For example:

if (nodeTag(groupexpr) == T_RelabelType)

usually, we use IsA() func.

--
Tender Wang

#9Tender Wang
tndrwang@gmail.com
In reply to: 狂奔的蜗牛 (#5)
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 equal
or 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

<https://wx.mail.qq.com/home/index?t=readmail_businesscard_midpage&amp;nocheck=true&amp;name=%E7%8B%82%E5%A5%94%E7%9A%84%E8%9C%97%E7%89%9B&amp;icon=https%3A%2F%2Fthirdqq.qlogo.cn%2Fg%3Fb%3Doidb%26k%3DF3ILZJoyBvhT7vey7fHs0w%26s%3D0&amp;mail=1105066510%40qq.com&amp;code=0OLolyhbnzdYmSQwz3uMNg5ZMBS6mWEqSZZTWmeoPRwBlPc_UEG58XLsX3F7BiIHbpXQLp6_Aqvg7Y4A-nAZLg&gt;

------------------ 原始邮件 ------------------
*发件人:* "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

Attachments:

v3-0001-fix-group_by_has_partkey-bug-and-add-regress-test.patchapplication/octet-stream; name=v3-0001-fix-group_by_has_partkey-bug-and-add-regress-test.patchDownload+77-4
#10狂奔的蜗牛
1105066510@qq.com
In reply to: Tender Wang (#9)
回复: BUG #18568: BUG: Result wrong when do group by on partition table!

this case will enter `if (IsA(groupexpr, RelabelType))` branch.&nbsp;

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&nbsp;eval_const_expressions_mutator() to transform CollateExpr to RelableType if CollateExpr-&gt;collOid not equal to Collate-&gt;arg's collation.

About V3 patch, PartCollMatchesExprColl() may be not&nbsp;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

&nbsp;

------------------&nbsp;原始邮件&nbsp;------------------
发件人: "Tender Wang" <tndrwang@gmail.com&gt;;
发送时间:&nbsp;2024年8月7日(星期三) 中午12:01
收件人:&nbsp;"狂奔的蜗牛"<1105066510@qq.com&gt;;
抄送:&nbsp;"pgsql-bugs"<pgsql-bugs@lists.postgresql.org&gt;;
主题:&nbsp;Re: BUG #18568: BUG: Result wrong when do group by on partition table!

I think what I have&nbsp;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&nbsp;set_baserel_partition_key_exprs() logic.

I look through your patch and make some&nbsp;changes.

1.&nbsp;
&nbsp; git am your patch, report warnings, some code format issue.
2.&nbsp;&nbsp;
&nbsp;I removed this branch: if (IsA(groupexpr, RelabelType)).

&nbsp;Because in your added test case, it didn't enter this branch. I didn't figure out what&nbsp; kind of group by clause is RelableType.
&nbsp;You can provide&nbsp;a test case based on the v3 patch.

3. Tweek a little about the test case.

By&nbsp;the way, your last two emails only sent to me, please cc the pgsql-bugs.

狂奔的蜗牛 <1105066510@qq.com&gt; 于2024年8月6日周二 19:33写道:

Partkey's collation&nbsp;stored in&nbsp;RelOptInfo-&gt;part_scheme, and I use it to fix the bug.
The attachment is my solution!

狂奔的蜗牛
1105066510@qq.com

&nbsp;

------------------&nbsp;原始邮件&nbsp;------------------
发件人: "Tender Wang" <tndrwang@gmail.com&gt;;
发送时间:&nbsp;2024年8月6日(星期二) 下午4:42
收件人:&nbsp;"狂奔的蜗牛"<1105066510@qq.com&gt;;"pgsql-bugs"<pgsql-bugs@lists.postgresql.org&gt;;

主题:&nbsp;Re: BUG #18568: BUG: Result wrong when do group by on partition table!

PG Bug reporting form <noreply@postgresql.org&gt; 于2024年8月6日周二 15:01写道:

The following bug has been logged on the website:

Bug reference:&nbsp; &nbsp; &nbsp; 18568
Logged by:&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; Webbo Han
Email address:&nbsp; &nbsp; &nbsp; 1105066510@qq.com
PostgreSQL version: 16.3
Operating system:&nbsp; &nbsp;centos 7.6
Description:&nbsp; &nbsp; &nbsp; &nbsp;

First, we create one case-insensitive collation use ICU:
```sql
&nbsp; &nbsp; &nbsp; &nbsp; CREATE COLLATION case_insensitive (
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; provider = icu,
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; locale = 'und-u-ks-level2',
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; deterministic = false
&nbsp; &nbsp; &nbsp; &nbsp; );
```

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
&nbsp; &nbsp; &nbsp; &nbsp; SET enable_partitionwise_aggregate TO true;
&nbsp; &nbsp; &nbsp; &nbsp; SET enable_partitionwise_join TO true;
&nbsp; &nbsp; &nbsp; &nbsp; SET max_parallel_workers_per_gather TO 0;
&nbsp; &nbsp; &nbsp; &nbsp; SET enable_incremental_sort TO off;
&nbsp; &nbsp; &nbsp; &nbsp; CREATE TABLE pagg_tab (c text collate case_insensitive) PARTITION BY LIST(c
collate "C");
&nbsp; &nbsp; &nbsp; &nbsp; CREATE TABLE pagg_tab_p1 PARTITION OF pagg_tab FOR VALUES IN ('a', 'b',
'c', 'd');
&nbsp; &nbsp; &nbsp; &nbsp; CREATE TABLE pagg_tab_p2 PARTITION OF pagg_tab FOR VALUES IN ('e', 'f',
'A');
&nbsp; &nbsp; &nbsp; &nbsp; CREATE TABLE pagg_tab_p3 PARTITION OF pagg_tab FOR VALUES IN ('B', 'C',
'D', 'E');
&nbsp; &nbsp; &nbsp; &nbsp; INSERT INTO pagg_tab SELECT substr('abcdeABCDE', (i % 10) +1 , 1) FROM
generate_series(0, 2999) i;
&nbsp; &nbsp; &nbsp; &nbsp; 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
&nbsp; &nbsp; &nbsp; &nbsp; postgres=# SELECT c collate case_insensitive, count(c) FROM pagg_tab GROUP
BY c collate case_insensitive;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;c | count
&nbsp; &nbsp; &nbsp; &nbsp; ---+-------
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;A |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;e |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;E |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;D |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;C |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;B |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;d |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;c |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;b |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;a |&nbsp; &nbsp;300
&nbsp; &nbsp; &nbsp; &nbsp; (10 rows)
```

The reason is the function group_by_has_partkey() do not check partkey's
collation, that lead to explain error.
```shell
&nbsp; &nbsp; &nbsp; &nbsp; postgres=# EXPLAIN SELECT c collate case_insensitive, count(c) FROM
pagg_tab GROUP BY c collate case_insensitive ;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; QUERY PLAN
&nbsp; &nbsp; &nbsp; &nbsp; --------------------------------------------------------------------------------------
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;Append&nbsp; (cost=12.00..60.15 rows=10 width=10)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; HashAggregate&nbsp; (cost=12.00..12.02 rows=2 width=10)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;Group Key: pagg_tab.c
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; Seq Scan on pagg_tab_p2 pagg_tab&nbsp; (cost=0.00..9.00 rows=600
width=2)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; HashAggregate&nbsp; (cost=24.00..24.04 rows=4 width=10)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;Group Key: pagg_tab_1.c
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; Seq Scan on pagg_tab_p3 pagg_tab_1&nbsp; (cost=0.00..18.00
rows=1200 width=2)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; HashAggregate&nbsp; (cost=24.00..24.04 rows=4 width=10)
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;Group Key: pagg_tab_2.c
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-&gt;&nbsp; Seq Scan on pagg_tab_p1 pagg_tab_2&nbsp; (cost=0.00..18.00
rows=1200 width=2)
&nbsp; &nbsp; &nbsp; &nbsp; (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&nbsp; 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-&gt;parttypcoll[cnt] value not&nbsp; partkey-&gt;partcollation value.

And partkey-&gt;parttypcoll[cnt] is assigned from pg_attribute , which is the column c meta data.&nbsp;
Should we use partkey-&gt;partcollation value?&nbsp; 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

#11Tender Wang
tndrwang@gmail.com
In reply to: 狂奔的蜗牛 (#10)
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

Attachments:

v4-0001-fix-group_by_has_partkey-bug-and-add-regress-test.patchapplication/octet-stream; name=v4-0001-fix-group_by_has_partkey-bug-and-add-regress-test.patchDownload+103-4
#12狂奔的蜗牛
1105066510@qq.com
In reply to: Tender Wang (#11)
回复: BUG #18568: BUG: Result wrong when do group by on partition table!

PartCollMatchesExprColl()&nbsp; 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,&nbsp;
becasuse if groupexpr is RelabelType and&nbsp; partcoll equal to groupcoll,&nbsp; the "equal(groupexpr, partexpr) &amp;&amp; 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

&nbsp;

------------------&nbsp;原始邮件&nbsp;------------------
发件人: "Tender Wang" <tndrwang@gmail.com&gt;;
发送时间:&nbsp;2024年8月7日(星期三) 下午2:57
收件人:&nbsp;"狂奔的蜗牛"<1105066510@qq.com&gt;;
抄送:&nbsp;"pgsql-bugs"<pgsql-bugs@lists.postgresql.org&gt;;"aleksander"<aleksander@timescale.com&gt;;
主题:&nbsp;Re: BUG #18568: BUG: Result wrong when do group by on partition table!

狂奔的蜗牛 <1105066510@qq.com&gt; 于2024年8月7日周三 13:35写道:

this case will enter `if (IsA(groupexpr, RelabelType))` branch.&nbsp;

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&nbsp;eval_const_expressions_mutator() to transform CollateExpr to RelableType if CollateExpr-&gt;collOid not equal to Collate-&gt;arg's collation.

Yech. Thanks for the explanation.
Because exprCollation() return the resultcollid if the nodetag is Relabletype, the expr() &amp;&amp; PartKeyCollMatchesExprColl() can handle this situation.
So we don't need "if (IsA(groupexpr, RelableTyple) " this branch.
&nbsp;

About V3 patch, PartCollMatchesExprColl() may be not&nbsp;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.&nbsp; I uesd it because partition prune do this way. I keep it temporarily.&nbsp;

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

#13Tender Wang
tndrwang@gmail.com
In reply to: 狂奔的蜗牛 (#12)
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

#14Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Tender Wang (#7)
Re: BUG #18568: BUG: Result wrong when do group by on partition table!

Hi,

On Tue, Aug 6, 2024 at 10:24 PM Tender Wang <tndrwang@gmail.com> wrote:

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");

I think above create table again. Should we allow this CREATE TABLE? The partition key
definition are not same with column definiton. Is it better to report error for users?

Not really. As the documentation says, collation can be specified per
column or per operation:

https://www.postgresql.org/docs/current/collation.html

In this case, the operation is partitioning. When you specify the
COLLATE clause for a partition key, it means that the partitioning
logic, such as partition tuple routing, will use that collation
instead of the column-specified or the column type's collation.

That's similar to how you can create an index on a column using a
different collation than the column's own:

create table foo (a text collate case_insensitive);
create index on foo (a collate "C");

I do notice that the CREATE TABLE documentation does not describe what
the COLLATE clause does when mentioned in a PARTITION BY clause.
We'll need to fix that.

--
Thanks, Amit Langote

#15Tender Wang
tndrwang@gmail.com
In reply to: Amit Langote (#14)
Re: BUG #18568: BUG: Result wrong when do group by on partition table!

Amit Langote <amitlangote09@gmail.com> 于2024年10月22日周二 15:33写道:

Hi,

On Tue, Aug 6, 2024 at 10:24 PM Tender Wang <tndrwang@gmail.com> wrote:

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");

I think above create table again. Should we allow this CREATE TABLE?

The partition key

definition are not same with column definiton. Is it better to report

error for users?

Not really. As the documentation says, collation can be specified per
column or per operation:

https://www.postgresql.org/docs/current/collation.html

In this case, the operation is partitioning. When you specify the
COLLATE clause for a partition key, it means that the partitioning
logic, such as partition tuple routing, will use that collation
instead of the column-specified or the column type's collation.

That's similar to how you can create an index on a column using a
different collation than the column's own:

create table foo (a text collate case_insensitive);
create index on foo (a collate "C");

Thanks for your explanation.

I do notice that the CREATE TABLE documentation does not describe what
the COLLATE clause does when mentioned in a PARTITION BY clause.
We'll need to fix that.

Agree.

--
Thanks,
Tender Wang

#16Tender Wang
tndrwang@gmail.com
In reply to: Amit Langote (#14)
Re: BUG #18568: BUG: Result wrong when do group by on partition table!

Hi Amit,

Amit Langote <amitlangote09@gmail.com> 于2024年10月22日周二 15:33写道:

Not really. As the documentation says, collation can be specified per
column or per operation:

https://www.postgresql.org/docs/current/collation.html

In this case, the operation is partitioning. When you specify the
COLLATE clause for a partition key, it means that the partitioning
logic, such as partition tuple routing, will use that collation
instead of the column-specified or the column type's collation.

Since you said partition key had its own collation, and but we used column
type's collation in
set_baserel_partition_key_exprs() as below:

partexpr = (Expr *) makeVar(varno, attno,
partkey->parttypid[cnt],
partkey->parttypmod[cnt],
partkey->parttypcoll[cnt], 0);

I think why not we directly use the partition key collation(e.g.
partcollation).

--
Thanks,
Tender Wang

#17Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Tender Wang (#16)
Re: BUG #18568: BUG: Result wrong when do group by on partition table!

On Tue, Oct 22, 2024 at 5:30 PM Tender Wang <tndrwang@gmail.com> wrote:

Amit Langote <amitlangote09@gmail.com> 于2024年10月22日周二 15:33写道:

Not really. As the documentation says, collation can be specified per
column or per operation:

https://www.postgresql.org/docs/current/collation.html

In this case, the operation is partitioning. When you specify the
COLLATE clause for a partition key, it means that the partitioning
logic, such as partition tuple routing, will use that collation
instead of the column-specified or the column type's collation.

Since you said partition key had its own collation, and but we used column type's collation in
set_baserel_partition_key_exprs() as below:

partexpr = (Expr *) makeVar(varno, attno,
partkey->parttypid[cnt],
partkey->parttypmod[cnt],
partkey->parttypcoll[cnt], 0);

I think why not we directly use the partition key collation(e.g. partcollation).

That's a good question but I don't immediately know the answer.

It seems like it has been like this since the beginning or since the
commit that added the RelOptInfo.partexprs field (9140cf8269).

--
Thanks, Amit Langote

#18Tender Wang
tndrwang@gmail.com
In reply to: Amit Langote (#17)
Re: BUG #18568: BUG: Result wrong when do group by on partition table!

Amit Langote <amitlangote09@gmail.com> 于2024年10月22日周二 17:25写道:

On Tue, Oct 22, 2024 at 5:30 PM Tender Wang <tndrwang@gmail.com> wrote:

Amit Langote <amitlangote09@gmail.com> 于2024年10月22日周二 15:33写道:

Not really. As the documentation says, collation can be specified per
column or per operation:

https://www.postgresql.org/docs/current/collation.html

In this case, the operation is partitioning. When you specify the
COLLATE clause for a partition key, it means that the partitioning
logic, such as partition tuple routing, will use that collation
instead of the column-specified or the column type's collation.

Since you said partition key had its own collation, and but we used

column type's collation in

set_baserel_partition_key_exprs() as below:

partexpr = (Expr *) makeVar(varno, attno,
partkey->parttypid[cnt],
partkey->parttypmod[cnt],
partkey->parttypcoll[cnt], 0);

I think why not we directly use the partition key collation(e.g.

partcollation).

That's a good question but I don't immediately know the answer.

It seems like it has been like this since the beginning or since the
commit that added the RelOptInfo.partexprs field (9140cf8269).

When I looked at the commit(9140cf8269), I found that in 9140cf8269, the
PartitionSchemeData struct had
a filed: Oid *parttypcoll; whose value was equal to column type's
collation. But now HEAD this field has changed to
Oid *partcollation; whose value is equal to partition key collation. This
commit 2af28e6 made above change.

commit 2af28e603319224e87fd35ab62f36ef6de45eaac
Author: Robert Haas <rhaas@postgresql.org>
Date: Wed Feb 28 12:16:09 2018 -0500

For partitionwise join, match on partcollation, not parttypcoll.

The previous code considered two tables to have the partition scheme
if the underlying columns had the same collation, but what we
actually need to compare is not the collations associated with the
column but the collation used for partitioning. Fix that.

Robert Haas and Amit Langote

Discussion:
/messages/by-id/0f95f924-0efa-4cf5-eb5f-9a3d1bc3c33d@lab.ntt.co.jp

Now I suspect the commit 2af28e6 forgot to fix RelOptInfo's partexprs
collation.
If we change the partexprs collation to Partition Key collation, as I said
in [1]/messages/by-id/CAHewXNnKLrZYG4iqaYw=uB3XWRrYRZHo7VtcMsbUEbdbajQg2Q@mail.gmail.com, this bug will not happen.
But I'm not sure this fix will affect other codes that use RelOptInfo's
partexprs.
Any thoughts?

[1]: /messages/by-id/CAHewXNnKLrZYG4iqaYw=uB3XWRrYRZHo7VtcMsbUEbdbajQg2Q@mail.gmail.com
/messages/by-id/CAHewXNnKLrZYG4iqaYw=uB3XWRrYRZHo7VtcMsbUEbdbajQg2Q@mail.gmail.com

--
Thanks,
Tender Wang

#19Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Tender Wang (#18)
Re: BUG #18568: BUG: Result wrong when do group by on partition table!

Hi,

On Tue, Oct 22, 2024 at 8:14 PM Tender Wang <tndrwang@gmail.com> wrote:

Amit Langote <amitlangote09@gmail.com> 于2024年10月22日周二 17:25写道:

On Tue, Oct 22, 2024 at 5:30 PM Tender Wang <tndrwang@gmail.com> wrote:

Amit Langote <amitlangote09@gmail.com> 于2024年10月22日周二 15:33写道:

Not really. As the documentation says, collation can be specified per
column or per operation:

https://www.postgresql.org/docs/current/collation.html

In this case, the operation is partitioning. When you specify the
COLLATE clause for a partition key, it means that the partitioning
logic, such as partition tuple routing, will use that collation
instead of the column-specified or the column type's collation.

Since you said partition key had its own collation, and but we used column type's collation in
set_baserel_partition_key_exprs() as below:

partexpr = (Expr *) makeVar(varno, attno,
partkey->parttypid[cnt],
partkey->parttypmod[cnt],
partkey->parttypcoll[cnt], 0);

I think why not we directly use the partition key collation(e.g. partcollation).

That's a good question but I don't immediately know the answer.

It seems like it has been like this since the beginning or since the
commit that added the RelOptInfo.partexprs field (9140cf8269).

When I looked at the commit(9140cf8269), I found that in 9140cf8269, the PartitionSchemeData struct had
a filed: Oid *parttypcoll; whose value was equal to column type's collation. But now HEAD this field has changed to
Oid *partcollation; whose value is equal to partition key collation. This commit 2af28e6 made above change.

commit 2af28e603319224e87fd35ab62f36ef6de45eaac
Author: Robert Haas <rhaas@postgresql.org>
Date: Wed Feb 28 12:16:09 2018 -0500

For partitionwise join, match on partcollation, not parttypcoll.

The previous code considered two tables to have the partition scheme
if the underlying columns had the same collation, but what we
actually need to compare is not the collations associated with the
column but the collation used for partitioning. Fix that.

Robert Haas and Amit Langote

Discussion: /messages/by-id/0f95f924-0efa-4cf5-eb5f-9a3d1bc3c33d@lab.ntt.co.jp

Good find.

Now I suspect the commit 2af28e6 forgot to fix RelOptInfo's partexprs collation.
If we change the partexprs collation to Partition Key collation, as I said in [1], this bug will not happen.
But I'm not sure this fix will affect other codes that use RelOptInfo's partexprs.

Yeah, I suspect we could maybe just put the collation from
partcollation into varcollid of the partition key Var, but what about
partition keys that are not simple column references? Would they
carry the correct collation such that exprCollation() returns the
right collation OID?

Also, we'll need to be sure that the semantic of anything that's using
partexprs is not broken because of this.

--
Thanks, Amit Langote

#20Tender Wang
tndrwang@gmail.com
In reply to: Amit Langote (#19)
Re: BUG #18568: BUG: Result wrong when do group by on partition table!

Amit Langote <amitlangote09@gmail.com> 于2024年10月22日周二 20:11写道:

Hi,

On Tue, Oct 22, 2024 at 8:14 PM Tender Wang <tndrwang@gmail.com> wrote:

Amit Langote <amitlangote09@gmail.com> 于2024年10月22日周二 17:25写道:

On Tue, Oct 22, 2024 at 5:30 PM Tender Wang <tndrwang@gmail.com> wrote:

Amit Langote <amitlangote09@gmail.com> 于2024年10月22日周二 15:33写道:

Not really. As the documentation says, collation can be specified

per

column or per operation:

https://www.postgresql.org/docs/current/collation.html

In this case, the operation is partitioning. When you specify the
COLLATE clause for a partition key, it means that the partitioning
logic, such as partition tuple routing, will use that collation
instead of the column-specified or the column type's collation.

Since you said partition key had its own collation, and but we used

column type's collation in

set_baserel_partition_key_exprs() as below:

partexpr = (Expr *) makeVar(varno, attno,
partkey->parttypid[cnt],
partkey->parttypmod[cnt],
partkey->parttypcoll[cnt], 0);

I think why not we directly use the partition key collation(e.g.

partcollation).

That's a good question but I don't immediately know the answer.

It seems like it has been like this since the beginning or since the
commit that added the RelOptInfo.partexprs field (9140cf8269).

When I looked at the commit(9140cf8269), I found that in 9140cf8269,

the PartitionSchemeData struct had

a filed: Oid *parttypcoll; whose value was equal to column type's

collation. But now HEAD this field has changed to

Oid *partcollation; whose value is equal to partition key collation.

This commit 2af28e6 made above change.

commit 2af28e603319224e87fd35ab62f36ef6de45eaac
Author: Robert Haas <rhaas@postgresql.org>
Date: Wed Feb 28 12:16:09 2018 -0500

For partitionwise join, match on partcollation, not parttypcoll.

The previous code considered two tables to have the partition scheme
if the underlying columns had the same collation, but what we
actually need to compare is not the collations associated with the
column but the collation used for partitioning. Fix that.

Robert Haas and Amit Langote

Discussion:

/messages/by-id/0f95f924-0efa-4cf5-eb5f-9a3d1bc3c33d@lab.ntt.co.jp

Good find.

Now I suspect the commit 2af28e6 forgot to fix RelOptInfo's partexprs

collation.

If we change the partexprs collation to Partition Key collation, as I

said in [1], this bug will not happen.

But I'm not sure this fix will affect other codes that use RelOptInfo's

partexprs.

Yeah, I suspect we could maybe just put the collation from
partcollation into varcollid of the partition key Var, but what about
partition keys that are not simple column references? Would they
carry the correct collation such that exprCollation() returns the
right collation OID?

The value of RelOptInfo's partexprs actually are assigned from
RelationData's rd_partkey.
And the rd_partkey is filled from pg_partitioned_table. The partcollation
also gets from pg_partitioned_table,
even though the partition key is not simple column reference, the collation
is correct.

Also, we'll need to be sure that the semantic of anything that's using
partexprs is not broken because of this.

Yeah. We should be careful.

--
Thanks,
Tender Wang

#21Tender Wang
tndrwang@gmail.com
In reply to: Amit Langote (#17)
#22Tender Wang
tndrwang@gmail.com
In reply to: Tender Wang (#21)
#23jian he
jian.universality@gmail.com
In reply to: Tender Wang (#22)
#24Tender Wang
tndrwang@gmail.com
In reply to: jian he (#23)
#25Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Tender Wang (#24)
#26Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Amit Langote (#25)
#27Tender Wang
tndrwang@gmail.com
In reply to: Amit Langote (#26)