Re: [BUGS] BUG #14785: Logical replication does not work after adding a column. Bug?

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