Re: Enable partitionwise join for partition keys wrapped by RelabelType

From: "Matheus Alcantara" <matheusssilv97(at)gmail(dot)com>
To: "Jan Nidzwetzki" <jan(at)planetscale(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-08-27 17:08:45
Message-ID: DKZW48ZFT3WO.PX88Z84ZQKNJ@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for the review. v5 is attached, now split into two patches: 0001 is
the partprune fix and 0002 is the exprs_known_equal() fix.

On Tue Aug 25, 2026 at 11:50 AM -03, Jan Nidzwetzki wrote:
> The new change in partprune.c might be something that deserves its own
> patch since it touches the partitioning code; it's a good improvement,
> and it is testable on its own (and not covered by tests so far).

Agreed, it's now 0001 with its own tests in partition_prune.sql. I
covered the three clause shapes the change affects: equality, IN-list
and IS NULL.

> The new test in partition_join.sql addresses the v3 regression, but it
> does not cover the behavior this patch changes: as you note above, with
> an expression partition key like ((c::text)), both sides keep the
> wrapper. So, the test produces identical output whether the patch is
> applied or not.
> [...]
> I think this would make a good test for partition_join.sql, since it
> fails without the patch.

Yes, you're right, thanks for catching this.

I used a slightly different test case with the same idea, mainly because
current_setting() isn't used anywhere else in partition_join.sql, so I'm
not sure if it could cause issues when running on buildfarm. pht3 and
pht4 are now hashed on (c, d), c is compared to a literal on both sides
so that its equality comes from the equivalence class, and d is joined
normally. Since hash pruning requires every partition key column to be
constrained, constraining only c prunes nothing, so all partitions
survive on both sides.

I confirmed that the new test fails without 0002. With the stripping
removed, the plan collapses to a Nested Loop over two Appends and
partition_join fails.

> However, the new plan is not necessarily faster than the old one.
> [...]
> I don't believe the patch is to blame here.
> [...]
> However, enabling partition-wise joins for these EC-derived equalities
> could negatively impact queries that previously benefited from run-time
> pruning. Was this interaction taken into account?

Yes, and I don't think the patch is the responsible here. Your analysis
seems correct to me. create_append_plan() builds the prunequal from
rel->baserestrictinfo, and build_join_rel() sets
joinrel->baserestrictinfo = NIL, so a partitionwise-join Append can
never carry run-time pruning. Worth adding that run-time pruning is
never costed either: part_prune_index is only set during plan creation,
after the cheapest path has already been chosen, and there's no cost
discount for it anywhere in costsize.c or pathnode.c. So both candidates
are costed as though they scan every partition, and the pruned plan's
real advantage is invisible to the comparison.

But that predates this patch since with a plain text partition key the
patch is a no-op, there's no RelabelType anywhere, so
exprs_known_equal() behaves exactly as before, and master already
prefers the partitionwise plan for your query shape, already giving up
run-time pruning. So it doesn't seem to me that the patch introduce any
hazard.

Teaching the cost model about run-time pruning, or letting partitionwise
Appends prune can be a way to improve this, but I'm not sure.

> 1) I couldn't find a test case that proves we need the collation guard
> in the introduced strip_collation_preserving_relabel() function. Do you
> have a test case that would break without the guard?

It's hard to reach because have_partkey_equi_join()'s own collation
check compares the partition key expression's collation against
partcollation -- it never learns the collation of the equivalence class
that proved the equality, since exprs_known_equal() returns only a
boolean and discards ec->ec_collation. So the partition key collation
has to equal the column's collation, with only the EC's collation
differing. That arises naturally: canonicalize_ec_expression()
deliberately wraps every EC member in a collation-changing RelabelType
so that it exposes the class's collation.

Under a non-deterministic collation the partitions are then not
join-closed, so a partitionwise join drops matching rows. I confirmed by
deleting the guard and rerunning the suite: the query returns 5 instead
of 9.

Note this tests the guard rather than the patch as a whole, so it also
passes on master, which doesn't strip at all. And only the em_expr side
is covered since I couldn't construct a partexpr carrying a top-level
collation-changing RelabelType, so the item1/item2 side is defensive
only.

> 2) I am wondering if the following cast in
> match_clause_to_partition_key() is actually needed, since
> RelabelType.arg is already an (Expr *):
>
> partkey = (const Expr *) ((const RelabelType *) partkey)->arg;

Right, the outer cast is redundant. Removed. Only the inner (const
RelabelType *) cast remains.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com

Attachment Content-Type Size
v5-0001-Fix-partition-pruning-for-partition-keys-wrapped-.patch text/plain 6.5 KB
v5-0002-Enable-partitionwise-join-for-partition-keys-wrap.patch text/plain 20.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2026-08-27 17:14:54 Re: Online enable/disable data checksums functions return success even when the launcher fails to start
Previous Message Ashutosh Bapat 2026-08-27 17:08:07 Re: PGQ catalog representation and pg_dump support