Re: hyrax vs. RelationBuildPartitionDesc

Thanks for the updated patch.

On Sat, Apr 13, 2019 at 4:47 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.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