Subject: Wrong result when enable_partitionwise_join is on if collation of PartitionKey and Column is different.

From: Tender Wang <[email-masked]>
Date: Wed, 23 Oct 2024 21:48:11 +0800

Hi all,

I find another issue as $SUBJECT when I work on [1].
I will use [1] SQL to show the problem.
CREATE COLLATION case_insensitive (
provider = icu,
locale = 'und-u-ks-level2',
deterministic = false
);
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;

set max_parallel_workers_per_gather = 0;
postgres=# show enable_partitionwise_join ;
 enable_partitionwise_join
---------------------------
 off
(1 row)
postgres=# explain select count(*) from pagg_tab t1 join pagg_tab t2 on
t1.c = t2.c;
                                         QUERY PLAN
--------------------------------------------------------------------------------------------
 Aggregate  (cost=24915.00..24915.01 rows=1 width=8)
   ->  Hash Join  (cost=97.50..20415.00 rows=1800000 width=0)
         Hash Cond: (t1.c = t2.c)
         ->  Append  (cost=0.00..60.00 rows=3000 width=2)
               ->  Seq Scan on pagg_tab_p2 t1_1  (cost=0.00..9.00 rows=600
width=2)
               ->  Seq Scan on pagg_tab_p3 t1_2  (cost=0.00..18.00
rows=1200 width=2)
               ->  Seq Scan on pagg_tab_p1 t1_3  (cost=0.00..18.00
rows=1200 width=2)
         ->  Hash  (cost=60.00..60.00 rows=3000 width=2)
               ->  Append  (cost=0.00..60.00 rows=3000 width=2)
                     ->  Seq Scan on pagg_tab_p2 t2_1  (cost=0.00..9.00
rows=600 width=2)
                     ->  Seq Scan on pagg_tab_p3 t2_2  (cost=0.00..18.00
rows=1200 width=2)
                     ->  Seq Scan on pagg_tab_p1 t2_3  (cost=0.00..18.00
rows=1200 width=2)
(12 rows)

postgres=#  select count(*) from pagg_tab t1 join pagg_tab t2 on t1.c =
t2.c;
  count
---------
 1800000
(1 row)

But if we set enable_partitionwise_join is on, the result is different, as
below.
postgres=# set enable_partitionwise_join = on;
SET
postgres=# explain select count(*) from pagg_tab t1 join pagg_tab t2 on
t1.c = t2.c;
                                         QUERY PLAN
--------------------------------------------------------------------------------------------
 Aggregate  (cost=17010.00..17010.01 rows=1 width=8)
   ->  Append  (cost=16.50..14760.00 rows=900000 width=0)
         ->  Hash Join  (cost=16.50..2052.00 rows=180000 width=0)
               Hash Cond: (t1_1.c = t2_1.c)
               ->  Seq Scan on pagg_tab_p2 t1_1  (cost=0.00..9.00 rows=600
width=2)
               ->  Hash  (cost=9.00..9.00 rows=600 width=2)
                     ->  Seq Scan on pagg_tab_p2 t2_1  (cost=0.00..9.00
rows=600 width=2)
         ->  Hash Join  (cost=33.00..4104.00 rows=360000 width=0)
               Hash Cond: (t1_2.c = t2_2.c)
               ->  Seq Scan on pagg_tab_p3 t1_2  (cost=0.00..18.00
rows=1200 width=2)
               ->  Hash  (cost=18.00..18.00 rows=1200 width=2)
                     ->  Seq Scan on pagg_tab_p3 t2_2  (cost=0.00..18.00
rows=1200 width=2)
         ->  Hash Join  (cost=33.00..4104.00 rows=360000 width=0)
               Hash Cond: (t1_3.c = t2_3.c)
               ->  Seq Scan on pagg_tab_p1 t1_3  (cost=0.00..18.00
rows=1200 width=2)
               ->  Hash  (cost=18.00..18.00 rows=1200 width=2)
                     ->  Seq Scan on pagg_tab_p1 t2_3  (cost=0.00..18.00
rows=1200 width=2)
(17 rows)

postgres=#  select count(*) from pagg_tab t1 join pagg_tab t2 on t1.c =
t2.c;
 count
--------
 900000
(1 row)

I think the root cause of this thread and [1] are same.  We don't use the
Partition Key collation but column's
collation to fill the RelOptInfo partexprs field in
set_baserel_partition_key_exprs().
If the Partition Key definition is same as column definition,  which most
times is,
that will be ok. But if it's not, this thread issue will arise.

As far as I know, only partition pruning logic considers not only call
equal(), but also check collation match.
Other codes only call equal() to check if the exprs match the partition key.
For example, in this thread case, match_expr_to_partition_keys() think the
expr match the partition key:
if (equal(lfirst(lc), expr))
    return cnt;

Although We can fix this issue like [1], I think why not directly use the
partkey->partcollation[cnt], which its value is
same with pg_partitioned_table's partcollation. I tried this to fix [1],
but at that time, I was unsure if it was the correct fix.

Until I find this thread issue, I think we should do it this way.
In the attached patch, I include this thread test and [1] test case.

I have two questions in my head:
1.  Does partition pruning logic still check the collation match with this
patch.
2. icu can work on all platform?

Any thoughts?

[1]
https://www.postgresql.org/message-id/18568-2a9afb6b9f7e6ed3%40postgresql.org
-- 
Thanks,
Tender Wang

---

From: Tender Wang <[email-masked]>
Date: Thu, 24 Oct 2024 12:46:23 +0800

> I find another issue as $SUBJECT when I work on [1].

When I continue to work on this, I find below issue. But I'm not sure
whether it is a bug.

postgres=# create table part_index(a text primary key) partition by list (
a collate "POSIX");
ERROR:  unique constraint on partitioned table must include all
partitioning columns
DETAIL:  PRIMARY KEY constraint on table "part_index" lacks column "a"
which is part of the partition key.
postgres=# create table part_index(a text) partition by list ( a collate
"POSIX");
CREATE TABLE
postgres=# alter table part_index add primary key (a);
ERROR:  unique constraint on partitioned table must include all
partitioning columns
DETAIL:  PRIMARY KEY constraint on table "part_index" lacks column "a"
which is part of the partition key.

It seems we can't create a primary key if the collation is different
between columnDef and PartitionKey.

By the way, I think the error message is misleading to users.
ostgres=# alter table part_index add primary key (a);
ERROR:  unique constraint on partitioned table must include all
partitioning columns
DETAIL:  PRIMARY KEY constraint on table "part_index" lacks column "a"
which is part of the partition key.

-- 
Thanks,
Tender Wang

---

From: Amit Langote <[email-masked]>
Date: Thu, 24 Oct 2024 15:33:13 +0900

Hi,

On Thu, Oct 24, 2024 at 1:46 PM Tender Wang <[email-masked]> wrote:
> When I continue to work on this, I find below issue. But I'm not sure whether it is a bug.
>
> postgres=# create table part_index(a text primary key) partition by list ( a collate "POSIX");
> ERROR:  unique constraint on partitioned table must include all partitioning columns
> DETAIL:  PRIMARY KEY constraint on table "part_index" lacks column "a" which is part of the partition key.
> postgres=# create table part_index(a text) partition by list ( a collate "POSIX");
> CREATE TABLE
> ERROR:  unique constraint on partitioned table must include all partitioning columns
> DETAIL:  PRIMARY KEY constraint on table "part_index" lacks column "a" which is part of the partition key.
>
> It seems we can't create a primary key if the collation is different between columnDef and PartitionKey.

Yeah, you don't want to have the PK index and the partitioning logic
to not be in sync about the collation rules applied to the individual
rows.

> ERROR:  unique constraint on partitioned table must include all partitioning columns
> DETAIL:  PRIMARY KEY constraint on table "part_index" lacks column "a" which is part of the partition key.

I think it's kind of similar to the message you get when a GROUP BY
column's collation doesn't match the column appearing in the SELECT
list:

explain SELECT c collate case_insensitive, count(c) FROM
pagg_tab_case_s GROUP BY c collate "C";
ERROR:  column "pagg_tab_case_s.c" must appear in the GROUP BY clause
or be used in an aggregate function
LINE 1: explain SELECT c collate case_insensitive, count(c) FROM pag...

Perhaps it would be more helpful for the error message or hint or
detail to mention the actual discrepancy (collation mismatch) that's
causing the error.

There might be other instances of such an error and I am not sure it
would be worthwhile to find and fix them all.

-- 
Thanks, Amit Langote

---

From: Amit Langote <[email-masked]>
Date: Fri, 1 Nov 2024 16:26:45 +0900

Hi,

On Wed, Oct 23, 2024 at 10:48 PM Tender Wang <[email-masked]> wrote:
> I think the root cause of this thread and [1] are same.  We don't use the Partition Key collation but column's
> collation to fill the RelOptInfo partexprs field in set_baserel_partition_key_exprs().
> If the Partition Key definition is same as column definition,  which most times is,
> As far as I know, only partition pruning logic considers not only call equal(), but also check collation match.
> For example, in this thread case, match_expr_to_partition_keys() think the expr match the partition key:
> Although We can fix this issue like [1], I think why not directly use the partkey->partcollation[cnt], which its value is
> same with pg_partitioned_table's partcollation. I tried this to fix [1], but at that time, I was unsure if it was the correct fix.

I think it would be better to keep RelOptInfo.partexprs unchanged in
these fixes.  I haven't been able to come up with a way to "assign"
the correct collation to partition keys that are not simple column
references.  Checking PartitionScheme.partcollation separately is
enough to fix these bugs and aligns with the style of existing code,
such as match_clause_to_partition_key() in partprune.c.

--
Thanks, Amit Langote