Re: Remove page-read callback from XLogReaderState.

How to reproduce:

 $ sed -i -E -e 's|^/\* #define WAL_DEBUG \*/$|#define WAL_DEBUG|' src/include/pg_config_manual.h
 $ ./configure && make

Expected: PostgreSQL is successfully made.

Actual: I got the following make error:

>>>>>>>>
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -O2 -I../../../../src/include  -D_GNU_SOURCE   -c -o xlog.o xlog.c
In file included from /usr/include/x86_64-linux-gnu/bits/types/stack_t.h:23,
                 from /usr/include/signal.h:303,
                 from ../../../../src/include/storage/sinval.h:17,
                 from ../../../../src/include/access/xact.h:22,
                 from ../../../../src/include/access/twophase.h:17,
                 from xlog.c:33:
xlog.c: In function ‘XLogInsertRecord’:
xlog.c:1219:56: error: called object is not a function or function pointer
 1219 |    debug_reader = XLogReaderAllocate(wal_segment_size, NULL NULL);
      |                                                        ^~~~
xlog.c:1219:19: error: too few arguments to function ‘XLogReaderAllocate’
 1219 |    debug_reader = XLogReaderAllocate(wal_segment_size, NULL NULL);
      |                   ^~~~~~~~~~~~~~~~~~
In file included from ../../../../src/include/access/clog.h:14,
                 from xlog.c:25:
../../../../src/include/access/xlogreader.h:243:25: note: declared here
  243 | extern XLogReaderState *XLogReaderAllocate(int wal_segment_size,
      |                         ^~~~~~~~~~~~~~~~~~
make[4]: *** [<builtin>: xlog.o] Error 1
<<<<<<<<

The following chunk in 0002 seems to be the cause of the error.  There is no comma between two NULLs.

>>>>>>>>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e570e56a24..f9b0108602 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
(..snipped..)
@@ -1225,8 +1218,7 @@ XLogInsertRecord(XLogRecData *rdata,
            appendBinaryStringInfo(&recordBuf, rdata->data, rdata->len);

        if (!debug_reader)
-           debug_reader = XLogReaderAllocate(wal_segment_size, NULL,
-                                             XL_ROUTINE(), NULL);
+           debug_reader = XLogReaderAllocate(wal_segment_size, NULL NULL);

        if (!debug_reader)
        {
<<<<<<<<


= 2. readBuf allocation in XLogReaderAllocate =

AFAIU, not XLogReaderAllocate() itself but its caller is now responsible for allocating XLogReaderState->readBuf.  However, the following code still remains in src/backend/access/transam/xlogreader.c:

>>>>>>>>
     74 XLogReaderState *
     75 XLogReaderAllocate(int wal_segment_size, const char *waldir,
     76                                    WALSegmentCleanupCB cleanup_cb)
     77 {
      :
     98     state->readBuf = (char *) palloc_extended(XLOG_BLCKSZ,
     99                                               MCXT_ALLOC_NO_OOM);
<<<<<<<<

Is this okay?


= 3. XLOG_FROM_ANY assigned to global readSource =
Regarding the following chunk in 0003:

>>>>>>>>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6b42d9015f..bcb4ef270f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -804,18 +804,14 @@ static XLogSegNo openLogSegNo = 0;
  * These variables are used similarly to the ones above, but for reading
  * the XLOG.  Note, however, that readOff generally represents the offset
  * of the page just read, not the seek position of the FD itself, which
- * will be just past that page. readLen indicates how much of the current
- * page has been read into readBuf, and readSource indicates where we got
- * the currently open file from.
+ * will be just past that page. readSource indicates where we got the
+ * currently open file from.
  * Note: we could use Reserve/ReleaseExternalFD to track consumption of
  * this FD too; but it doesn't currently seem worthwhile, since the XLOG is
  * not read by general-purpose sessions.
  */
 static int readFile = -1;
-static XLogSegNo readSegNo = 0;
-static uint32 readOff = 0;
-static uint32 readLen = 0;
-static XLogSource readSource = XLOG_FROM_ANY;
+static XLogSource readSource = 0;  /* XLOG_FROM_* code */

 /*
  * Keeps track of which source we're currently reading from. This is
<<<<<<<<

I think it is better to keep the line "static XLogSource readSource = XLOG_FROM_ANY;".  XLOG_FROM_ANY is already defined as 0 in src/backend/access/transam/xlog.c.


Regards,
Takashi