On Fri, Jan 20, 2017 at 4:30 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Stephen Frost <sfr...@snowman.net> writes:
>> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>>> Robert Haas <robertmh...@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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to