| From: | Jan Nidzwetzki <jan(at)planetscale(dot)com> |
|---|---|
| To: | Matheus Alcantara <matheusssilv97(at)gmail(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Enable partitionwise join for partition keys wrapped by RelabelType |
| Date: | 2026-09-01 14:01:06 |
| Message-ID: | 6b029a87-82af-407f-831c-25db6a0ec719@planetscale.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hello Matheus,
Thanks for the updated (v7) set of these patches. The set applies
cleanly to master and check-world runs without any failures.
On 31.08.26 20:07, Matheus Alcantara wrote:
>
> Your 0003 patch looks correct to me, I've just made two additions to its
> test:
>
> 1. The fix strips both rel->partexprs and rel->nullable_partexprs, but a
> two-way LEFT JOIN only ever consults the non-nullable list, so half the
> change had no coverage. I added a three-way case:
>
> SELECT count(*) FROM (pht5 t1 LEFT JOIN pht6 t2 ON t1.c::text = t2.c::text)
> LEFT JOIN pht7 t3 ON t2.c::text = t3.c::text;
Patches 1-3
===========
Your analysis regarding distribute_qual_to_rels() / maybe_equivalence
sounds reasonable to me. Thanks as well for adding more test coverage to
patch 3. I verified the nullable_partexprs point by reverting only that
branch of the strip. The two-way case still passes, while the three-way
case goes fully non-partitionwise. So, the new test covers the half that
mine missed.
I also looked for other places in the same family that might still be
missing the stripping (e.g., RIGHT, SEMI, and ANTI joins), and didn't
find any. So patches 1 to 3 look good to me from my side.
Patch 4
=======
> While checking whether match_expr_to_partition_keys(), I found one more
> case, and I've fixed on 0004. group_by_has_partkey() strips RelabelType
> from the grouping expressions but not from the partition key expression.
> The effect is that full partitionwise aggregation is never chosen for a
> partition key involving a binary-compatible cast, and the plan falls
> back to partial aggregation with a finalize step. Both spellings fail,
> for slightly different reasons: GROUP BY c compares a bare Var against a
> RelabelType, and GROUP BY c::text strips the grouping side to a bare Var
> while the partition key side stays wrapped.
I think I found a problem in patch 4. group_by_has_partkey() has no
opfamily check, so stripping the RelabelType from the partition key side
means the grouping uses the argument type's equality. In contrast, the
partitioning uses the result type's. If the argument type has coarser
equality than the target, a group can span partitions, and full
partitionwise aggregation produces the wrong result.
For example:
CREATE EXTENSION citext;
CREATE TABLE ct (a int, c citext) PARTITION BY LIST ((c::text));
CREATE TABLE ct_1 PARTITION OF ct FOR VALUES IN ('A');
CREATE TABLE ct_2 PARTITION OF ct FOR VALUES IN ('a');
INSERT INTO ct SELECT i, (CASE WHEN i%2=0 THEN 'A' ELSE 'a' END)::citext
FROM generate_series(1,2000) i;
ANALYZE ct;
SET enable_partitionwise_aggregate = on;
SELECT c, count(*) FROM ct GROUP BY c;
c | count
---+-------
A | 1000
a | 1000
(2 rows)
SET enable_partitionwise_aggregate = off;
SELECT c, count(*) FROM ct GROUP BY c;
c | count
---+-------
A | 2000
(1 row)
The partitions split 'A' and 'a' by text equality, while the grouping
merges them by citext equality. I confirmed that patch 4 introduces
this. Reverting only the planner.c change gives the correct result.
So, I think we need to add an opfamily check here.
Version 8
=========
I attached a new version of the patch series. It contains patch 3 with
an improved commit message. When I wrote the old one, I thought we would
merge it. But if it stays separate, it deserves a better message. Apart
from the commit message, patches 1 to 3 are unchanged.
In patch 4, I added an op_in_opfamily() check in group_by_has_partkey().
This check requires the grouping clause's equality operator to be a
member of the partitioning operator family, which mirrors what
have_partkey_equi_join() already does for the clause operator. The
problem I described above is fixed with that change.
What do you think?
Best regards
Jan
--
Jan Nidzwetzki
PlanetScale Postgres Core Team
| Attachment | Content-Type | Size |
|---|---|---|
| v8-0001-Fix-partition-pruning-for-partition-keys-wrapped-.patch | text/plain | 6.5 KB |
| v8-0002-Enable-partitionwise-join-for-partition-keys-wrap.patch | text/plain | 20.8 KB |
| v8-0003-Enable-partitionwise-join-for-outer-joins-on-Rela.patch | text/plain | 10.3 KB |
| v8-0004-Enable-full-partitionwise-aggregate-for-partition.patch | text/plain | 12.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Sehrope Sarkuni | 2026-09-01 14:04:13 | Re: [PATCH] Speed up pg_waldump TAP test and fix some GitHub CI Windows flakiness |
| Previous Message | Fujii Masao | 2026-09-01 13:56:01 | Re: [PATCH] doc: clarify AS requirement when VALUES used in a FROM clause |