On 16.11.2018 11:23, Michael Paquier wrote:
On Thu, Nov 15, 2018 at 06:12:38PM +0900, Kyotaro HORIGUCHI wrote:
This patch eliminates unnecessary copying that was done for
non-continued records. Now the return value of XLogReadRecord
directly points into page buffer holded in XLogReaderStats. It is
safe because no caller site uses the returned pointer beyond the
replacement of buffer content at the next call to the same
function.
I was looking at this patch, and shouldn't we worry about compatibility
with plugins or utilities which look directly at what's in readRecordBuf
for the record contents? Let's not forget that the contents of
XLogReaderState are public.
According to my experience, I clarify some comments to avoid this
mistakes in the future (see attachment).
--
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company
>From 7de252405ef5d5979fe2711515c0c6402abc0e06 Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" <a.lepik...@postgrespro.ru>
Date: Mon, 19 Nov 2018 07:08:28 +0500
Subject: [PATCH] Some clarifications on readRecordBuf comments
---
src/backend/access/transam/xlogreader.c | 4 ++--
src/include/access/xlogreader.h | 6 +++++-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index c5e019bf77..88d0bcf48a 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -209,8 +209,8 @@ allocate_recordbuf(XLogReaderState *state, uint32 reclength)
* If the reading fails for some other reason, NULL is also returned, and
* *errormsg is set to a string with details of the failure.
*
- * The returned pointer (or *errormsg) points to an internal buffer that's
- * valid until the next call to XLogReadRecord.
+ * The returned pointer (or *errormsg) points to an internal read-only buffer
+ * that's valid until the next call to XLogReadRecord.
*/
XLogRecord *
XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index 40116f8ecb..0837625b95 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -185,7 +185,11 @@ struct XLogReaderState
*/
TimeLineID nextTLI;
- /* Buffer for current ReadRecord result (expandable) */
+ /*
+ * Buffer for current ReadRecord result (expandable).
+ * Used in the case, than current ReadRecord result don't fit on the
+ * currently read WAL page.
+ */
char *readRecordBuf;
uint32 readRecordBufSize;
--
2.17.1
