Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 9/25/17 15:16, Andres Freund wrote:
>> This'll accept tablenames like pg_temp_1foo, right? Might be worth
>> being a bit narrower in the test.
> Committed with that change. Thanks.
This patch is using the wrong approach entirely. Every other place in
the backend that is trying to exclude temp relations uses a test on the
containing namespace, not the relname. You should be using
RELATION_IS_OTHER_TEMP(), or if there's good reason to decide this
without opening the relation, maybe you could get away with
isOtherTempNamespace(get_rel_namespace(relid)).
I also note that as committed, the patch will dump core on a
concurrently-dropped relation, because get_rel_name returns NULL
under such circumstances.
BTW, get_rel_sync_entry has some other serious problems: it is not being
at all careful about whether persistent data structures are left in sane
states if it gets an error partway through. In particular it'll leave
behind a new hash entry in entirely-unknown state, and if LoadPublications
gets an error, it will also leave a time bomb behind in the form of
not nil, but already-list-freed, data->publications. And I sure do not
understand why a single static variable publications_valid is being used
to remember validity of data->publications ... couldn't there be more
than one of those?
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs