On Thu, Dec 22, 2022 at 1:15 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Thu, 22 Dec 2022 12:35:46 +0530, Amit Kapila <amit.kapila16@gmail.com> wrote in
> > I have addressed these comments in the attached. Additionally, I have
> > modified the docs and commit messages to make those clear. I think
> > instead of adding new tests with this patch, it may be better to
> > change some of the existing tests related to streaming to use this
> > parameter as that will clearly show one of the purposes of this patch.
>
> Being late but I'm warried that we might sometime regret that the lack
> of the explicit default. Don't we really need it?
>
For this, I like your proposal for "buffered" as an explicit default value.
> + Allows streaming or serializing changes immediately in logical decoding.
> + The allowed values of <varname>logical_decoding_mode</varname> are the
> + empty string and <literal>immediate</literal>. When set to
> + <literal>immediate</literal>, stream each change if
> + <literal>streaming</literal> option is enabled, otherwise, serialize
> + each change. When set to an empty string, which is the default,
> + decoding will stream or serialize changes when
> + <varname>logical_decoding_work_mem</varname> is reached.
>
> With (really) fresh eyes, I took a bit long time to understand what
> the "streaming" option is. Couldn't we augment the description by a
> bit?
>
Okay, how about modifying it to: "When set to
<literal>immediate</literal>, stream each change if
<literal>streaming</literal> option (see optional parameters set by
CREATE SUBSCRIPTION) is enabled, otherwise, serialize each change.
--
With Regards,
Amit Kapila.
