Re: hyrax vs. RelationBuildPartitionDesc

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: hyrax vs. RelationBuildPartitionDesc
Date: 2019-04-13 15:32:09
Message-ID: CA+HiwqF=toO_==HRN6FDXSZE4DExo+PxX3xPNuBs6UAdF3WLPQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for the updated patch.

On Sat, Apr 13, 2019 at 4:47 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > As a parenthetical note, I observe that relcache.c seems to know
> > almost nothing about rd_partcheck. rd_partkey and rd_partdesc both
> > have handling in RelationClearRelation(), but rd_partcheck does not,
> > and I suspect that's wrong. So the problems are probably not confined
> > to the relcache-drop-time problem that you observed.
>
> I concluded that that's not parenthetical but pretty relevant...

Hmm, do you mean we should grow the same "keep_" logic for
rd_partcheck as we have for rd_partkey and rd_partdesc? I don't see
it in the updated patch though.

> Attached is a revised version of Amit's patch at [1] that makes these
> data structures be treated more similarly. I also added some Asserts
> and comment improvements to address the complaints I made upthread about
> under-documentation of all this logic.

Thanks for all the improvements.

> I also cleaned up the problem the code had with failing to distinguish
> "partcheck list is NIL" from "partcheck list hasn't been computed yet".
> For a relation with no such constraints, generate_partition_qual would do
> the full pushups every time.

Actually, callers must have checked that the table is a partition
(relispartition). It wouldn't be a bad idea to add an Assert or elog
in generate_partition_qual.

> I'm not sure if the case actually occurs
> commonly enough that that's a performance problem, but failure to account
> for it made my added assertions fall over :-( and I thought fixing it
> was better than weakening the assertions.

Hmm, I wonder why the Asserts failed given what I said above.

> I haven't made back-patch versions yet. I'd expect they could be
> substantially the same, except the two new fields have to go at the
> end of struct RelationData to avoid ABI breaks.

To save you the pain of finding the right files to patch in
back-branches, I made those (attached).

Thanks,
Amit

Attachment Content-Type Size
pg10-fix-rd_partcheck-management-2.patch application/octet-stream 9.4 KB
pg11-fix-rd_partcheck-management-2.patch application/octet-stream 9.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-04-13 15:53:39 Re: hyrax vs. RelationBuildPartitionDesc
Previous Message Tom Lane 2019-04-13 15:09:00 Re: Useless code in RelationCacheInitializePhase3