Improve relcache invalidation handling of currently invisible relations.

The corner case where a relcache invalidation tried to rebuild the
entry for a referenced relation but couldn't find it in the catalog
wasn't correct.

The code tried to RelationCacheDelete/RelationDestroyRelation the
entry. That didn't work when assertions are enabled because the latter
contains an assertion ensuring the refcount is zero. It's also more
generally a bad idea, because by virtue of being referenced somebody
might actually look at the entry, which is possible if the error is
trapped and handled via a subtransaction abort.

Instead just error out, without deleting the entry. As the entry is
marked invalid, the worst that can happen is that the invalid (and at
some point unused) entry lingers in the relcache.

Discussion: 22459.1418656530@sss.pgh.pa.us

There should be no way to hit this case < 9.4 where logical decoding
introduced a bug that can hit this. But since the code for handling
the corner case is there it should do something halfway sane, so
backpatch all the the way back.  The logical decoding bug will be
handled in a separate commit.

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 10d300a3e8804753794736d443bf32bbc6b86fbd..b7cda3e17339ffd62519c1f6a3fb1dd9abc96714 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -2101,9 +2101,13 @@ RelationClearRelation(Relation relation, bool rebuild)
                newrel = RelationBuildDesc(save_relid, false);
                if (newrel == NULL)
                {
-                       /* Should only get here if relation was deleted */
-                       RelationCacheDelete(relation);
-                       RelationDestroyRelation(relation, false);
+                       /*
+                        * This shouldn't happen as dropping a relation is intended to be
+                        * impossible if still referenced (c.f. CheckTableNotInUse()). But
+                        * if we get here anyway, we can't just delete the relcache entry,
+                        * as it possibly could get accessed later (as e.g. the error
+                        * might get trapped and handled via a subtransaction rollback).
+                        */
                        elog(ERROR, "relation %u deleted while still in use", save_relid);
                }