Fix RelationIdGetRelation calls that weren't bothering with error checks.

Some of these are quite old, but that doesn't make them not bugs.
We'd rather report a failure via elog than SIGSEGV.

While at it, uniformly spell the error check as !RelationIsValid(rel)
rather than a bare rel == NULL test.  The machine code is the same
but it seems better to be consistent.

Coverity complained about this today, not sure why, because the
mistake is in fact old.

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index e7c32f2a13235ee7de79000e714e4cc0ca53c182..917033f3f6fcb521da04d792153a8bec0f4b7a1d 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1553,7 +1553,7 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
 
                    relation = RelationIdGetRelation(reloid);
 
-                   if (relation == NULL)
+                   if (!RelationIsValid(relation))
                        elog(ERROR, "could not open relation with OID %u (for filenode \"%s\")",
                             reloid,
                             relpathperm(change->data.tp.relnode,
@@ -1671,7 +1671,7 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
 
                            relation = RelationIdGetRelation(relid);
 
-                           if (relation == NULL)
+                           if (!RelationIsValid(relation))
                                elog(ERROR, "could not open relation with OID %u", relid);
 
                            if (!RelationIsLogicallyLogged(relation))
@@ -3031,6 +3031,10 @@ ReorderBufferToastReplace(ReorderBuffer *rb, ReorderBufferTXN *txn,
    desc = RelationGetDescr(relation);
 
    toast_rel = RelationIdGetRelation(relation->rd_rel->reltoastrelid);
+   if (!RelationIsValid(toast_rel))
+       elog(ERROR, "could not open relation with OID %u",
+            relation->rd_rel->reltoastrelid);
+
    toast_desc = RelationGetDescr(toast_rel);
 
    /* should we allocate from stack instead? */