Re: Make default subscription streaming option as Parallel

On Mon, Oct 7, 2024 at 4:03 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Mon, 7 Oct 2024 at 12:26, Hayato Kuroda (Fujitsu)
> <kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> > One comment for your patch;
> > Shouldn't we add a streaming=off case for pg_dump test? You added lines but no one
> > passes it.
> >
>
> Update existing pg_dump tests to cover the streaming options, the
> attached patch has the changes for the same.
>

@@ -5235,6 +5235,8 @@ dumpSubscription(Archive *fout, const
SubscriptionInfo *subinfo)
appendPQExpBufferStr(query, ", streaming = on");
else if (strcmp(subinfo->substream, "p") == 0)
appendPQExpBufferStr(query, ", streaming = parallel");
+ else
+ appendPQExpBufferStr(query, ", streaming = off");

For newer versions (>=18), we shouldn't need to specify "streaming =
parallel" while creating a subscription as that will be the default.
However, with the above code pg_dump statements will still have that.
There is nothing wrong with that but ideally, it won't be required.
Now, OTOH, doing that would require some version-handling code, which
is nothing new for pg_dump but not sure it makes sense for this
particular case. Another related point is that normally we don't
recommend people to use dump generated with pg_dump to use with lower
server versions than pg_dump itself, but the current proposed patch
will allow that. However, if we change it as I am saying that won't be
possible. So, I am okay with the current code but want to see if
anyone else thinks otherwise or if I am missing something.

--
With Regards,
Amit Kapila.