On Fri, Jan 20, 2017 at 4:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Stephen Frost <sfrost@snowman.net> writes:
>> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>>> Robert Haas <robertmhaas@gmail.com> writes:
>>>> Hmm. That's bad. I kind of wonder how sane it is to think that we
>>>> can invoke SQL-callable functions during a relcache reload,
>
>>> You're doing WHAT?
>
>> Uh. +1.
>
> Now that I've calmed down a bit: the right way to do this sort of thing is
> simply to flush the invalidated data during reload, and recompute it when
> it is next requested, which necessarily will be inside a valid
> transaction. Compare e.g. the handling of the lists of a relation's
> indexes.
The existing handling of partition descriptors is modeled on and very
similar to the existing handling for other types of objects:
keep_tupdesc = equalTupleDescs(relation->rd_att,
newrel->rd_att); keep_rules = equalRuleLocks(relation->rd_rules,
newrel->rd_rules); keep_policies = equalRSDesc(relation->rd_rsdesc,
newrel->rd_rsdesc); keep_partkey = (relation->rd_partkey != NULL); keep_partdesc =
equalPartitionDescs(relation->rd_partkey,
relation->rd_partdesc,
newrel->rd_partdesc);
And I think the reason is the same too, namely, if we've got a pointer
into partition descriptor in the relcache, we don't want that to
suddenly get swapped out and replaced with a pointer to an equivalent
data structure at a different address, because then our pointer will
be dangling. That seems fine as far as it goes.
The difference is that those other equalBLAH functions call a
carefully limited amount of code whereas, in looking over the
backtrace you sent, I realized that equalPartitionDescs is calling
partition_bounds_equal which does this:
cmpval =
DatumGetInt32(FunctionCall2Coll(&key->partsupfunc[j],
key->partcollation[j],
b1->datums[i][j],
b2->datums[i][j]))
That's of course opening up a much bigger can of worms. But apart
from the fact that it's unsafe, I think it's also wrong, as I said
upthread. I think calling datumIsEqual() there should be better all
around. Do you think that's unsafe here?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
