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 |
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 |