Fix nbtree posting list update desc output.
authorPeter Geoghegan <[email protected]>
Mon, 10 Apr 2023 18:15:41 +0000 (11:15 -0700)
committerPeter Geoghegan <[email protected]>
Mon, 10 Apr 2023 18:15:41 +0000 (11:15 -0700)
We cannot use the generic array_desc approach with per-tuple nbtree
posting list update metadata because array_desc can only deal with fixed
width elements (e.g., page offset numbers).  Using array_desc led to
incorrect rmgr descriptions for updates from nbtree DELETE/VACUUM WAL
records.

To fix, add specialized code to describe the update metadata as array
elements in desc output.  We now iterate over the update metadata using
an approach that matches related REDO routines.

Also stop showing the updates offset number array separately in nbtree
DELETE/VACUUM desc output.  It's redundant information, since the same
page offset numbers appear in the description of each individual update
element.  Also make some small tweaks to the way that we format arrays
in all desc routines (not just nbtree desc routines) to make arrays a
little less verbose.

Oversight in commit 1c453cfd, which enhanced the nbtree rmgr desc
routines.

Author: Peter Geoghegan <[email protected]>
Discussion: https://postgr.es/m/CAH2-WzkbYuvwYKm-Y-72QEh6SPMQcAo9uONv+mR3bMGcu9E_Cg@mail.gmail.com

doc/src/sgml/pgwalinspect.sgml
src/backend/access/rmgrdesc/heapdesc.c
src/backend/access/rmgrdesc/nbtdesc.c
src/backend/access/rmgrdesc/rmgrdesc_utils.c
src/include/access/rmgrdesc_utils.h

index b3712be0097d03a2ca1e5da24e7bbbd2f475170b..a1a4422fb42c8d0c1e98418048df959b0f688140 100644 (file)
       after the <replaceable>in_lsn</replaceable> argument.  For
       example:
 <screen>
-postgres=# SELECT * FROM pg_get_wal_record_info('0/E84F5E8');
--[ RECORD 1 ]----+--------------------------------------------------
-start_lsn        | 0/E84F5E8
-end_lsn          | 0/E84F620
-prev_lsn         | 0/E84F5A8
+postgres=# SELECT * FROM pg_get_wal_record_info('0/E419E28');
+-[ RECORD 1 ]----+-------------------------------------------------
+start_lsn        | 0/E419E28
+end_lsn          | 0/E419E68
+prev_lsn         | 0/E419D78
 xid              | 0
 resource_manager | Heap2
 record_type      | VACUUM
-record_length    | 50
+record_length    | 58
 main_data_length | 2
 fpi_length       | 0
-description      | nunused: 1, unused: [ 22 ]
-block_ref        | blkref #0: rel 1663/16389/20884 fork main blk 126
+description      | nunused: 5, unused: [1, 2, 3, 4, 5]
+block_ref        | blkref #0: rel 1663/16385/1249 fork main blk 364
 </screen>
      </para>
      <para>
@@ -144,18 +144,18 @@ block_ref        |
       references.  Returns one row per block reference per WAL record.
       For example:
 <screen>
-postgres=# SELECT * FROM pg_get_wal_block_info('0/10E9D80', '0/10E9DC0');
+postgres=# SELECT * FROM pg_get_wal_block_info('0/1230278', '0/12302B8');
 -[ RECORD 1 ]-----+-----------------------------------
-start_lsn         | 0/10E9D80
-end_lsn           | 0/10E9DC0
-prev_lsn          | 0/10E9860
+start_lsn         | 0/1230278
+end_lsn           | 0/12302B8
+prev_lsn          | 0/122FD40
 block_id          | 0
 reltablespace     | 1663
 reldatabase       | 1
-relfilenode       | 2690
+relfilenode       | 2658
 relforknumber     | 0
-relblocknumber    | 5
-xid               | 117
+relblocknumber    | 11
+xid               | 341
 resource_manager  | Btree
 record_type       | INSERT_LEAF
 record_length     | 64
@@ -163,8 +163,8 @@ main_data_length  | 2
 block_data_length | 16
 block_fpi_length  | 0
 block_fpi_info    |
-description       | off 14
-block_data        | \x00005400020010001407000000000000
+description       | off: 46
+block_data        | \x00002a00070010402630000070696400
 block_fpi_data    |
 </screen>
      </para>
index 6dfd7d7d1305b66bba4e69c44e9cc124c0951047..3bd083875207c44d5b0eee1983afc8da426757a9 100644 (file)
@@ -127,7 +127,7 @@ heap_desc(StringInfo buf, XLogReaderState *record)
        appendStringInfo(buf, ", nrelids: %u", xlrec->nrelids);
        appendStringInfoString(buf, ", relids:");
        array_desc(buf, xlrec->relids, sizeof(Oid), xlrec->nrelids,
-                  &relid_desc, NULL);
+                  &oid_elem_desc, NULL);
    }
    else if (info == XLOG_HEAP_CONFIRM)
    {
index 6bc0432650f614027e579dae4eac957f24d9b2c0..0db3274dc512328d08d304fd0c772077c62d2620 100644 (file)
 #include "access/nbtxlog.h"
 #include "access/rmgrdesc_utils.h"
 
-static void btree_del_desc(StringInfo buf, char *block_data, uint16 ndeleted,
-                          uint16 nupdated);
-static void btree_update_elem_desc(StringInfo buf, void *update, void *data);
-
-static void
-btree_del_desc(StringInfo buf, char *block_data, uint16 ndeleted,
-              uint16 nupdated)
-{
-   OffsetNumber *updatedoffsets;
-   xl_btree_update *updates;
-   OffsetNumber *data = (OffsetNumber *) block_data;
-
-   appendStringInfoString(buf, ", deleted:");
-   array_desc(buf, data, sizeof(OffsetNumber), ndeleted,
-              &offset_elem_desc, NULL);
-
-   appendStringInfoString(buf, ", updated:");
-   array_desc(buf, data, sizeof(OffsetNumber), nupdated,
-              &offset_elem_desc, NULL);
-
-   if (nupdated <= 0)
-       return;
-
-   updatedoffsets = (OffsetNumber *)
-       ((char *) data + ndeleted * sizeof(OffsetNumber));
-   updates = (xl_btree_update *) ((char *) updatedoffsets +
-                                  nupdated *
-                                  sizeof(OffsetNumber));
-
-   appendStringInfoString(buf, ", updates:");
-   array_desc(buf, updates, sizeof(xl_btree_update),
-              nupdated, &btree_update_elem_desc,
-              &updatedoffsets);
-}
-
-static void
-btree_update_elem_desc(StringInfo buf, void *update, void *data)
-{
-   xl_btree_update *new_update = (xl_btree_update *) update;
-   OffsetNumber *updated_offset = *((OffsetNumber **) data);
-
-   appendStringInfo(buf, "{ updated offset: %u, ndeleted tids: %u",
-                    *updated_offset, new_update->ndeletedtids);
-
-   appendStringInfoString(buf, ", deleted tids:");
-
-   array_desc(buf, (char *) new_update + SizeOfBtreeUpdate, sizeof(uint16),
-              new_update->ndeletedtids, &uint16_elem_desc, NULL);
-
-   updated_offset++;
-
-   appendStringInfo(buf, " }");
-}
+static void delvacuum_desc(StringInfo buf, char *block_data,
+                          uint16 ndeleted, uint16 nupdated);
 
 void
 btree_desc(StringInfo buf, XLogReaderState *record)
@@ -114,9 +63,8 @@ btree_desc(StringInfo buf, XLogReaderState *record)
                                 xlrec->ndeleted, xlrec->nupdated);
 
                if (!XLogRecHasBlockImage(record, 0))
-                   btree_del_desc(buf, XLogRecGetBlockData(record, 0, NULL),
+                   delvacuum_desc(buf, XLogRecGetBlockData(record, 0, NULL),
                                   xlrec->ndeleted, xlrec->nupdated);
-
                break;
            }
        case XLOG_BTREE_DELETE:
@@ -128,16 +76,15 @@ btree_desc(StringInfo buf, XLogReaderState *record)
                                 xlrec->ndeleted, xlrec->nupdated);
 
                if (!XLogRecHasBlockImage(record, 0))
-                   btree_del_desc(buf, XLogRecGetBlockData(record, 0, NULL),
+                   delvacuum_desc(buf, XLogRecGetBlockData(record, 0, NULL),
                                   xlrec->ndeleted, xlrec->nupdated);
-
                break;
            }
        case XLOG_BTREE_MARK_PAGE_HALFDEAD:
            {
                xl_btree_mark_page_halfdead *xlrec = (xl_btree_mark_page_halfdead *) rec;
 
-               appendStringInfo(buf, "topparent: %u; leaf: %u; left: %u; right: %u",
+               appendStringInfo(buf, "topparent: %u, leaf: %u, left: %u, right: %u",
                                 xlrec->topparent, xlrec->leafblk, xlrec->leftblk, xlrec->rightblk);
                break;
            }
@@ -146,11 +93,11 @@ btree_desc(StringInfo buf, XLogReaderState *record)
            {
                xl_btree_unlink_page *xlrec = (xl_btree_unlink_page *) rec;
 
-               appendStringInfo(buf, "left: %u; right: %u; level: %u; safexid: %u:%u; ",
+               appendStringInfo(buf, "left: %u, right: %u, level: %u, safexid: %u:%u, ",
                                 xlrec->leftsib, xlrec->rightsib, xlrec->level,
                                 EpochFromFullTransactionId(xlrec->safexid),
                                 XidFromFullTransactionId(xlrec->safexid));
-               appendStringInfo(buf, "leafleft: %u; leafright: %u; leaftopparent: %u",
+               appendStringInfo(buf, "leafleft: %u, leafright: %u, leaftopparent: %u",
                                 xlrec->leafleftsib, xlrec->leafrightsib,
                                 xlrec->leaftopparent);
                break;
@@ -242,3 +189,59 @@ btree_identify(uint8 info)
 
    return id;
 }
+
+static void
+delvacuum_desc(StringInfo buf, char *block_data,
+              uint16 ndeleted, uint16 nupdated)
+{
+   OffsetNumber *deletedoffsets;
+   OffsetNumber *updatedoffsets;
+   xl_btree_update *updates;
+
+   /* Output deleted page offset number array */
+   appendStringInfoString(buf, ", deleted:");
+   deletedoffsets = (OffsetNumber *) block_data;
+   array_desc(buf, deletedoffsets, sizeof(OffsetNumber), ndeleted,
+              &offset_elem_desc, NULL);
+
+   /*
+    * Output updates as an array of "update objects", where each element
+    * contains a page offset number from updated array.  (This is not the
+    * most literal representation of the underlying physical data structure
+    * that we could use.  Readability seems more important here.)
+    */
+   appendStringInfoString(buf, ", updated: [");
+   updatedoffsets = (OffsetNumber *) (block_data + ndeleted *
+                                      sizeof(OffsetNumber));
+   updates = (xl_btree_update *) ((char *) updatedoffsets +
+                                  nupdated *
+                                  sizeof(OffsetNumber));
+   for (int i = 0; i < nupdated; i++)
+   {
+       OffsetNumber off = updatedoffsets[i];
+
+       Assert(OffsetNumberIsValid(off));
+       Assert(updates->ndeletedtids > 0);
+
+       appendStringInfo(buf, "{ off: %u, nptids: %u, ptids: [",
+                        off, updates->ndeletedtids);
+       for (int p = 0; p < updates->ndeletedtids; p++)
+       {
+           uint16     *ptid;
+
+           ptid = (uint16 *) ((char *) updates + SizeOfBtreeUpdate) + p;
+           appendStringInfo(buf, "%u", *ptid);
+
+           if (p < updates->ndeletedtids - 1)
+               appendStringInfoString(buf, ", ");
+       }
+       appendStringInfoString(buf, "] }");
+       if (i < nupdated - 1)
+           appendStringInfoString(buf, ", ");
+
+       updates = (xl_btree_update *)
+           ((char *) updates + SizeOfBtreeUpdate +
+            updates->ndeletedtids * sizeof(uint16));
+   }
+   appendStringInfoString(buf, "]");
+}
index c95a41a254b236b2288f70869f58032b7e86b582..3d16b1fcba3138be48425d3e80d62f43f35034e2 100644 (file)
  * or comma, however all subsequent appends to the string are responsible for
  * prepending themselves with a comma followed by a space.
  *
- * Arrays should have a space between the opening square bracket and first
- * element and between the last element and closing brace.
- *
  * Flags should be in ALL CAPS.
  *
  * For lists/arrays of items, the number of those items should be listed at
  * the beginning with all of the other numbers.
  *
- * List punctuation should still be included even if there are 0 items.
- *
  * Composite objects in a list should be surrounded with { }.
  */
 void
@@ -51,16 +46,14 @@ array_desc(StringInfo buf, void *array, size_t elem_size, int count,
        appendStringInfoString(buf, " []");
        return;
    }
-   appendStringInfo(buf, " [");
+   appendStringInfoString(buf, " [");
    for (int i = 0; i < count; i++)
    {
-       if (i > 0)
-           appendStringInfoString(buf, ",");
-       appendStringInfoString(buf, " ");
-
        elem_desc(buf, (char *) array + elem_size * i, data);
+       if (i < count - 1)
+           appendStringInfoString(buf, ", ");
    }
-   appendStringInfoString(buf, " ]");
+   appendStringInfoString(buf, "]");
 }
 
 void
@@ -78,13 +71,7 @@ redirect_elem_desc(StringInfo buf, void *offset, void *data)
 }
 
 void
-relid_desc(StringInfo buf, void *relid, void *data)
+oid_elem_desc(StringInfo buf, void *relid, void *data)
 {
    appendStringInfo(buf, "%u", *(Oid *) relid);
 }
-
-void
-uint16_elem_desc(StringInfo buf, void *value, void *data)
-{
-   appendStringInfo(buf, "%u", *(uint16 *) value);
-}
index 13552d6f350c3c788515d323309b22dbe8c56c2b..00e614649064d9168c5013c910e32a481ff3c77a 100644 (file)
@@ -17,7 +17,6 @@ extern void array_desc(StringInfo buf, void *array, size_t elem_size, int count,
                       void *data);
 extern void offset_elem_desc(StringInfo buf, void *offset, void *data);
 extern void redirect_elem_desc(StringInfo buf, void *offset, void *data);
-extern void relid_desc(StringInfo buf, void *relid, void *data);
-extern void uint16_elem_desc(StringInfo buf, void *value, void *data);
+extern void oid_elem_desc(StringInfo buf, void *relid, void *data);
 
 #endif                         /* RMGRDESC_UTILS_H */