PS_NITPICKS_V5.txt

text/plain

Filename: PS_NITPICKS_V5.txt
Type: text/plain
Part: 0
Message: Re: Add contrib/pg_logicalsnapinspect
diff --git a/contrib/pg_logicalinspect/pg_logicalinspect.c b/contrib/pg_logicalinspect/pg_logicalinspect.c
index dc9041a..2111202 100644
--- a/contrib/pg_logicalinspect/pg_logicalinspect.c
+++ b/contrib/pg_logicalinspect/pg_logicalinspect.c
@@ -1,13 +1,17 @@
 /*-------------------------------------------------------------------------
  *
  * pg_logicalinspect.c
- *		  Functions to inspect contents of PostgreSQL logical snapshots
+ *		Functions to inspect contents of PostgreSQL logical snapshots
  *
  * Copyright (c) 2024, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *		  contrib/pg_logicalinspect/pg_logicalinspect.c
+ *		contrib/pg_logicalinspect/pg_logicalinspect.c
  *
+ *
+ * NOTES
+ * 		For any code change or issue fix here, it is highly recommended to
+ * 		give a thought about doing the same in SnapBuildRestore() as well.
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
@@ -23,18 +27,12 @@ PG_FUNCTION_INFO_V1(pg_get_logical_snapshot_meta);
 PG_FUNCTION_INFO_V1(pg_get_logical_snapshot_info);
 
 /*
- * NOTE: For any code change or issue fix here, it is highly recommended to
- * give a thought about doing the same in SnapBuildRestore() as well.
- */
-
-/*
  * Validate the logical snapshot file.
  */
 static void
 ValidateSnapshotFile(XLogRecPtr lsn, SnapBuildOnDisk *ondisk, const char *path)
 {
 	int			fd;
-	Size		sz;
 	pg_crc32c	checksum;
 	MemoryContext context;
 
@@ -63,7 +61,6 @@ ValidateSnapshotFile(XLogRecPtr lsn, SnapBuildOnDisk *ondisk, const char *path)
 	fsync_fname(path, false);
 	fsync_fname(PG_LOGICAL_SNAPSHOTS_DIR, true);
 
-
 	/* read statically sized portion of snapshot */
 	SnapBuildRestoreContents(fd, (char *) ondisk, SnapBuildOnDiskConstantSize, path);
 
@@ -93,7 +90,7 @@ ValidateSnapshotFile(XLogRecPtr lsn, SnapBuildOnDisk *ondisk, const char *path)
 	/* restore committed xacts information */
 	if (ondisk->builder.committed.xcnt > 0)
 	{
-		sz = sizeof(TransactionId) * ondisk->builder.committed.xcnt;
+		Size sz = sizeof(TransactionId) * ondisk->builder.committed.xcnt;
 		ondisk->builder.committed.xip = MemoryContextAllocZero(ondisk->builder.context, sz);
 		SnapBuildRestoreContents(fd, (char *) ondisk->builder.committed.xip, sz, path);
 		COMP_CRC32C(checksum, ondisk->builder.committed.xip, sz);
@@ -102,7 +99,7 @@ ValidateSnapshotFile(XLogRecPtr lsn, SnapBuildOnDisk *ondisk, const char *path)
 	/* restore catalog modifying xacts information */
 	if (ondisk->builder.catchange.xcnt > 0)
 	{
-		sz = sizeof(TransactionId) * ondisk->builder.catchange.xcnt;
+		Size sz = sizeof(TransactionId) * ondisk->builder.catchange.xcnt;
 		ondisk->builder.catchange.xip = MemoryContextAllocZero(ondisk->builder.context, sz);
 		SnapBuildRestoreContents(fd, (char *) ondisk->builder.catchange.xip, sz, path);
 		COMP_CRC32C(checksum, ondisk->builder.catchange.xip, sz);
@@ -210,12 +207,11 @@ pg_get_logical_snapshot_info(PG_FUNCTION_ARGS)
 	if (ondisk.builder.committed.xcnt > 0)
 	{
 		Datum	   *arrayelems;
-		int			narrayelems;
+		int			narrayelems = 0;
 
 		arrayelems = (Datum *) palloc(ondisk.builder.committed.xcnt * sizeof(Datum));
-		narrayelems = 0;
 
-		for (narrayelems = 0; narrayelems < ondisk.builder.committed.xcnt; narrayelems++)
+		for (; narrayelems < ondisk.builder.committed.xcnt; narrayelems++)
 			arrayelems[narrayelems] = Int64GetDatum((int64) ondisk.builder.committed.xip[narrayelems]);
 
 		values[i++] = PointerGetDatum(construct_array_builtin(arrayelems, narrayelems, INT8OID));
@@ -228,12 +224,11 @@ pg_get_logical_snapshot_info(PG_FUNCTION_ARGS)
 	if (ondisk.builder.catchange.xcnt > 0)
 	{
 		Datum	   *arrayelems;
-		int			narrayelems;
+		int			narrayelems = 0;
 
 		arrayelems = (Datum *) palloc(ondisk.builder.catchange.xcnt * sizeof(Datum));
-		narrayelems = 0;
 
-		for (narrayelems = 0; narrayelems < ondisk.builder.catchange.xcnt; narrayelems++)
+		for (; narrayelems < ondisk.builder.catchange.xcnt; narrayelems++)
 			arrayelems[narrayelems] = Int64GetDatum((int64) ondisk.builder.catchange.xip[narrayelems]);
 
 		values[i++] = PointerGetDatum(construct_array_builtin(arrayelems, narrayelems, INT8OID));
diff --git a/doc/src/sgml/pglogicalinspect.sgml b/doc/src/sgml/pglogicalinspect.sgml
index 7767de2..3cc7742 100644
--- a/doc/src/sgml/pglogicalinspect.sgml
+++ b/doc/src/sgml/pglogicalinspect.sgml
@@ -10,7 +10,7 @@
  <para>
   The <filename>pg_logicalinspect</filename> module provides SQL functions
   that allow you to inspect the contents of logical decoding components. It
-  allows to inspect serialized logical snapshots of a running
+  allows the inspection of serialized logical snapshots of a running
   <productname>PostgreSQL</productname> database cluster, which is useful
   for debugging or educational purposes.
  </para>
@@ -38,7 +38,7 @@
       the <filename>pg_logical/snapshots</filename> directory.
       The <replaceable>in_lsn</replaceable> argument can be extracted from the
       snapshot file name.
-      example:
+      For example:
 <screen>
 postgres=# SELECT * FROM pg_ls_logicalsnapdir();
 -[ RECORD 1 ]+-----------------------
@@ -79,7 +79,7 @@ version  | 6
       the <filename>pg_logical/snapshots</filename> directory.
       The <replaceable>in_lsn</replaceable> argument can be extracted from the
       snapshot file name.
-      example:
+      For example:
 <screen>
 postgres=# SELECT * FROM pg_ls_logicalsnapdir();
 -[ RECORD 1 ]+-----------------------