Re: A problem about partitionwise join

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Richard Guo <riguo(at)pivotal(dot)io>
Cc: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Amit Langote <amitlangote09(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: A problem about partitionwise join
Date: 2020-04-04 20:37:58
Message-ID: 502.1586032678@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Richard Guo <riguo(at)pivotal(dot)io> writes:
> Rebased the patch with latest master and also addressed the test case
> failure reported by PostgreSQL Patch Tester.

I looked this patch over, but I don't like it too much: it seems very
brute-force (and badly under-commented). Creating all those extra
RestrictInfos isn't too cheap in itself, plus they'll jam up the
equivalence-class machinery for future tests.

There is already something in equivclass.c that would almost do what
we want here: exprs_known_equal() would tell us whether the partkeys
can be found in the same eclass, without having to generate data
structures along the way. The current implementation is not watertight
because it doesn't check opclass semantics, but that consideration
can be bolted on readily enough. So that leads me to something like
the attached.

One argument that could be made against this approach is that if there
are a lot of partkey expressions, this requires O(N^2) calls to
exprs_known_equal, something that's already not too cheap. I think
that that's not a big problem because the number of partkey expressions
would only be equal to the join degree (ie it doesn't scale with the
number of partitions of the baserels) ... but maybe I'm wrong about
that? I also wonder if it's really necessary to check every pair
of partkey expressions. It seems at least plausible that in the
cases we care about, all the partkeys on each side would be in the same
eclasses anyway, so that comparing the first members of each list would
be sufficient. But I haven't beat on that point.

regards, tom lane

Attachment Content-Type Size
v3-0001-Fix-up-partitionwise-join.patch text/x-diff 14.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jaime Casanova 2020-04-04 21:03:52 Failed Assertion about PolymorphicType
Previous Message Andres Freund 2020-04-04 19:58:23 idea: reduce logical slot induced bloat when multiple databases are used