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 d5cba9a13c8d059eb618cb14b20e9893b9c20976..1d9844924138b001a76a974d5e697a4aaa6809ee 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1951,9 +1951,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);
+                       /*
+                        * 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);
                }