Accidental setting of XLogReaderState.private_data ?
StartupDecodingContext() initializes ctx->reader->private_data with ctx, and
it even does so twice. I couldn't find a place in the code where the
(LogicalDecodingContext *) pointer is retrieved from the reader, and a simple
test of logical replication works if the patch below is applied. Thus I assume
that assignment is a thinko, isn't it?
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Attachments:
xlogreader_private_data.difftext/x-diffDownload
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index 424fe86a1b..fbca486b59 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -172,14 +172,12 @@ StartupDecodingContext(List *output_plugin_options,
ctx->slot = slot;
- ctx->reader = XLogReaderAllocate(wal_segment_size, read_page, ctx);
+ ctx->reader = XLogReaderAllocate(wal_segment_size, read_page, NULL);
if (!ctx->reader)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
- ctx->reader->private_data = ctx;
-
ctx->reorder = ReorderBufferAllocate();
ctx->snapshot_builder =
AllocateSnapshotBuilder(ctx->reorder, xmin_horizon, start_lsn,
Antonin Houska <ah@cybertec.at> writes:
StartupDecodingContext() initializes ctx->reader->private_data with ctx, and
it even does so twice. I couldn't find a place in the code where the
(LogicalDecodingContext *) pointer is retrieved from the reader, and a simple
test of logical replication works if the patch below is applied. Thus I assume
that assignment is a thinko, isn't it?
Hmm. The second, duplicate assignment is surely pointless, but I think
that setting the ctx as the private_data is a good idea. It hardly seems
out of the question that it might be needed in future.
regards, tom lane
On Mon, Apr 15, 2019 at 11:06:18AM -0400, Tom Lane wrote:
Hmm. The second, duplicate assignment is surely pointless, but I think
that setting the ctx as the private_data is a good idea. It hardly seems
out of the question that it might be needed in future.
Agreed that we should keep the assignment done with
XLogReaderAllocate(). I have committed the patch which removes the
useless assignment though.
--
Michael