On Wed, Mar 15, 2017 at 2:40 PM, Andres Freund <and...@anarazel.de> wrote: > reviewing some citus code copied from postgres I noticed that > RemoveRelations() has the following bit: > > /* > * These next few steps are a great deal like > relation_openrv, but we > * don't bother building a relcache entry since we don't need > it. > * > * Check for shared-cache-inval messages before trying to > access the > * relation. This is needed to cover the case where the name > * identifies a rel that has been dropped and recreated since > the > * start of our transaction: if we don't flush the old > syscache entry, > * then we'll latch onto that entry and suffer an error later. > */ > AcceptInvalidationMessages(); > > /* Look up the appropriate relation using namespace search. */ > state.relkind = relkind; > state.heapOid = InvalidOid; > state.concurrent = drop->concurrent; > relOid = RangeVarGetRelidExtended(rel, lockmode, true, > > false, > > RangeVarCallbackForDropRelation, > > (void *) &state); > > which doesn't seem to make sense - RangeVarGetRelidExtended does > invalidation handling on it's own. > > Looks like this was left there in the course of > http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=2ad36c4e44c8b513f6155656e1b7a8d26715bb94 > > ISTM AcceptInvalidationMessages() and preceding comment should just be > removed?
Yeah, I don't think that would hurt anything. (I'm not sure it'll help anything either - the overhead of an extra AcceptInvalidationMessages() call is quite minimal - but, as you say, maybe it's worth doing just to discourage future code authors from including unnecessary fluff.) -- 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
