Date: Tue, 17 Mar 2020 16:24:48 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Arseny Sher <a.sher@postgrespro.ru>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: logical copy_replication_slot issues
Message-ID: <20200317192448.GA14979@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CA+fd4k5qWBLTqomDxfeEeLR-yBf9_+EHxfFcpnP8hc7Bg15OEg@mail.gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

Thanks Arseny and Masahiko, I pushed this patch just now.  I changed
some comments while at it, hopefully they are improvements.

On 2020-Mar-09, Masahiko Sawada wrote:

>     ctx = CreateInitDecodingContext(plugin, NIL,
> -                                   false,  /* do not build snapshot */
> +                                   false,  /* do not build data snapshot */
>                                     restart_lsn,
>                                     logical_read_local_xlog_page, NULL, NULL,
>                                     NULL);
> 
> I'm not sure this change makes the comment better. Could you elaborate
> on the motivation of this change?

I addressed this issue by adding a comment in CreateInitDecodingContext
to explain the parameter, and then reference that comment's terminology
in this call.  I think it ends up clearer overall -- not that this whole
area is at all particularly clear.

Thanks again.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services