Amit Langote <amitlangote09@gmail.com> writes:
> On Sat, Apr 13, 2019 at 4:47 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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.
No, the "keep_" stuff is only necessary when we're trying to preserve
data structures in-place, which is only important if non-relcache
code might be using pointers to it. Since rd_partcheck is never
directly accessed by external code, only copied, there can't be any
live pointers to it to worry about. Besides which, since it's load
on demand rather than something that RelationBuildDesc forces to be
valid immediately, any comparison would be guaranteed to fail: the
field in the new reldesc will always be empty at this point.
Perhaps there's an argument that it should be load-immediately not
load-on-demand, but that would be an optimization not a bug fix,
and I'm skeptical that it'd be an improvement anyway.
Probably this is something to revisit whenever somebody gets around to
addressing the whole copy-vs-dont-copy-vs-use-a-refcount business that
we were handwaving about upthread.
>> 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).
That does not mean that it's guaranteed to have any partcheck constraints;
there are counterexamples in the regression tests. It looks like the main
case is a LIST-partitioned table that has only a default partition.
regards, tom lane