On Thu, Dec 22, 2022 at 1:15 PM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > At Thu, 22 Dec 2022 12:35:46 +0530, Amit Kapila <amit.kapil...@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.
