Accidental setting of XLogReaderState.private_data ?

Started by Antonin Houskaalmost 7 years ago3 messageshackers
Jump to latest
#1Antonin Houska
ah@cybertec.at

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+1-3
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Antonin Houska (#1)
Re: Accidental setting of XLogReaderState.private_data ?

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

#3Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#2)
Re: Accidental setting of XLogReaderState.private_data ?

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