ReadBuffer() error checking - Mailing list pgsql-patches

AFAIK, ReadBuffer() will elog on error, so callers can assume that the
buffer it returns is valid. The vast majority of ReadBuffer() call sites
make this assumption, but some went to the trouble of checking that the
returned buffer was valid and elog'ing if it was not. I've removed the
error checking from the latter since it is dead code.

I thought about adding an assertion (or even a precautionary
elog(ERROR)) to ReadBuffer to verify that the returned buffer is indeed
valid, but I didn't end up doing it. Feel free to raise your hand if you
think this is a good idea.

Barring any objections, I'll apply the attached patch to HEAD tomorrow.

-Neil
# Old manifest: 92128e273bd6b087288df4682774aa91c04a3142
# New manifest: 04cf2873af05d24e9f9d64ea0c0ffb329c383c04
# Summary of changes:
#
#   patch src/backend/access/heap/heapam.c
#    from 35dbe0979f6da3660c24d7c771c536d1b9654804
#      to f09a58420b1e164abe3acec04f67728fbfc5275f
#
#   patch src/backend/commands/analyze.c
#    from f45c8a8b8aa9ddea9f11c0ac81766c70459765cf
#      to 6a6d58ea5ad5bbe78e8929d383248d589e55a20d
#
#   patch src/backend/commands/sequence.c
#    from 3875f769215f8b3cb4688b7d8b404c7c07d0f577
#      to d491c79eb9f33cdafcc6ac477051068eff49a656
#
#   patch src/backend/commands/trigger.c
#    from 3b939c53de75beddf950261f31b6538c366f6d37
#      to ba6f66c7449a2bb99807e481199ced25f9af79d9
#
--- src/backend/access/heap/heapam.c
+++ src/backend/access/heap/heapam.c
@@ -191,8 +191,6 @@
         *buffer = ReleaseAndReadBuffer(*buffer,
                                        relation,
                                        ItemPointerGetBlockNumber(tid));
-        if (!BufferIsValid(*buffer))
-            elog(ERROR, "ReadBuffer failed");

         LockBuffer(*buffer, BUFFER_LOCK_SHARE);

@@ -226,8 +224,6 @@
         *buffer = ReleaseAndReadBuffer(*buffer,
                                        relation,
                                        page);
-        if (!BufferIsValid(*buffer))
-            elog(ERROR, "ReadBuffer failed");

         LockBuffer(*buffer, BUFFER_LOCK_SHARE);

@@ -266,8 +264,6 @@
         *buffer = ReleaseAndReadBuffer(*buffer,
                                        relation,
                                        page);
-        if (!BufferIsValid(*buffer))
-            elog(ERROR, "ReadBuffer failed");

         LockBuffer(*buffer, BUFFER_LOCK_SHARE);

@@ -360,8 +358,6 @@
         *buffer = ReleaseAndReadBuffer(*buffer,
                                        relation,
                                        page);
-        if (!BufferIsValid(*buffer))
-            elog(ERROR, "ReadBuffer failed");

         LockBuffer(*buffer, BUFFER_LOCK_SHARE);
         dp = (Page) BufferGetPage(*buffer);
@@ -941,11 +939,6 @@
     buffer = ReleaseAndReadBuffer(*userbuf, relation,
                                   ItemPointerGetBlockNumber(tid));

-    if (!BufferIsValid(buffer))
-        elog(ERROR, "ReadBuffer(\"%s\", %lu) failed",
-             RelationGetRelationName(relation),
-             (unsigned long) ItemPointerGetBlockNumber(tid));
-
     /*
      * Need share lock on buffer to examine tuple commit status.
      */
@@ -1049,14 +1044,7 @@
      * get the buffer from the relation descriptor Note that this does a
      * buffer pin.
      */
-
     buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(tid));
-
-    if (!BufferIsValid(buffer))
-        elog(ERROR, "ReadBuffer(\"%s\", %lu) failed",
-             RelationGetRelationName(relation),
-             (unsigned long) ItemPointerGetBlockNumber(tid));
-
     LockBuffer(buffer, BUFFER_LOCK_SHARE);

     /*
@@ -1304,10 +1297,6 @@
     Assert(ItemPointerIsValid(tid));

     buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(tid));
-
-    if (!BufferIsValid(buffer))
-        elog(ERROR, "ReadBuffer failed");
-
     LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);

     dp = (PageHeader) BufferGetPage(buffer);
@@ -1528,8 +1524,6 @@
     Assert(ItemPointerIsValid(otid));

     buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(otid));
-    if (!BufferIsValid(buffer))
-        elog(ERROR, "ReadBuffer failed");
     LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);

     dp = (PageHeader) BufferGetPage(buffer);
@@ -1863,10 +1861,6 @@
     int            result;

     *buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(tid));
-
-    if (!BufferIsValid(*buffer))
-        elog(ERROR, "ReadBuffer failed");
-
     LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);

     dp = (PageHeader) BufferGetPage(*buffer);
--- src/backend/commands/analyze.c
+++ src/backend/commands/analyze.c
@@ -811,8 +811,6 @@
          * tuples.
          */
         targbuffer = ReadBuffer(onerel, targblock);
-        if (!BufferIsValid(targbuffer))
-            elog(ERROR, "ReadBuffer failed");
         LockBuffer(targbuffer, BUFFER_LOCK_SHARE);
         targpage = BufferGetPage(targbuffer);
         maxoffset = PageGetMaxOffsetNumber(targpage);
--- src/backend/commands/sequence.c
+++ src/backend/commands/sequence.c
@@ -192,10 +192,6 @@
     /* Initialize first page of relation with special magic number */

     buf = ReadBuffer(rel, P_NEW);
-
-    if (!BufferIsValid(buf))
-        elog(ERROR, "ReadBuffer failed");
-
     Assert(BufferGetBlockNumber(buf) == 0);

     page = (PageHeader) BufferGetPage(buf);
@@ -850,9 +846,6 @@
     Form_pg_sequence seq;

     *buf = ReadBuffer(rel, 0);
-    if (!BufferIsValid(*buf))
-        elog(ERROR, "ReadBuffer failed");
-
     LockBuffer(*buf, BUFFER_LOCK_EXCLUSIVE);

     page = (PageHeader) BufferGetPage(*buf);
--- src/backend/commands/trigger.c
+++ src/backend/commands/trigger.c
@@ -1625,9 +1625,6 @@

         buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(tid));

-        if (!BufferIsValid(buffer))
-            elog(ERROR, "ReadBuffer failed");
-
         dp = (PageHeader) BufferGetPage(buffer);
         lp = PageGetItemId(dp, ItemPointerGetOffsetNumber(tid));