Thread: Fix ALTER SUBSCRIPTION ... SET PUBLICATION documentation
Hi,
When I read the documentation of ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (...),
it says "set_publication_option" only support "refresh" in documentation [1].
However, we can also supply the "copy_data" option, and the code is:
case ALTER_SUBSCRIPTION_PUBLICATION:
{
bool copy_data;
bool refresh;
parse_subscription_options(stmt->options,
NULL, /* no "connect" */
NULL, NULL, /* no "enabled" */
NULL, /* no "create_slot" */
NULL, NULL, /* no "slot_name" */
©_data,
NULL, /* no "synchronous_commit" */
&refresh,
NULL, NULL, /* no "binary" */
NULL, NULL); /* no "streaming" */
values[Anum_pg_subscription_subpublications - 1] =
publicationListToArray(stmt->publication);
replaces[Anum_pg_subscription_subpublications - 1] = true;
update_tuple = true;
/* Refresh if user asked us to. */
if (refresh)
{
if (!sub->enabled)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("ALTER SUBSCRIPTION with refresh is not allowed for disabled subscriptions"),
errhint("Use ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (refresh = false).")));
/* Make sure refresh sees the new list of publications. */
sub->publications = stmt->publication;
AlterSubscription_refresh(sub, copy_data);
}
break;
}
Should we fix the documentation or the code? I'd be inclined fix the documentation.
[1] - https://www.postgresql.org/docs/devel/sql-altersubscription.html
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
Attachment
On Tue, Jan 26, 2021 at 4:56 PM japin <japinli@hotmail.com> wrote: > > > Hi, > > When I read the documentation of ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (...), > it says "set_publication_option" only support "refresh" in documentation [1]. > However, we can also supply the "copy_data" option, and the code is: > I think there is a reference to the 'copy_data' option as well. There is a sentence saying: "Additionally, refresh options as described under REFRESH PUBLICATION may be specified." and then if you Refresh option, there we do mention about 'copy_data', isn't that sufficient? -- With Regards, Amit Kapila.
On Wed, Jan 27, 2021 at 4:57 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Tue, Jan 26, 2021 at 4:56 PM japin <japinli@hotmail.com> wrote: > > > > > > Hi, > > > > When I read the documentation of ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (...), > > it says "set_publication_option" only support "refresh" in documentation [1]. > > However, we can also supply the "copy_data" option, and the code is: > > > > I think there is a reference to the 'copy_data' option as well. There > is a sentence saying: "Additionally, refresh options as described > under REFRESH PUBLICATION may be specified." and then if you Refresh > option, there we do mention about 'copy_data', isn't that sufficient? Right. It looks like the copy_option is indirectly mentioned with the statement "Additionally, refresh options as described under REFRESH PUBLICATION may be specified." under "set_publication_option". IMHO, we can keep it that way. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
On Wed, 27 Jan 2021 at 19:47, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: > On Wed, Jan 27, 2021 at 4:57 PM Amit Kapila <amit.kapila16@gmail.com> wrote: >> >> On Tue, Jan 26, 2021 at 4:56 PM japin <japinli@hotmail.com> wrote: >> > >> > >> > Hi, >> > >> > When I read the documentation of ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (...), >> > it says "set_publication_option" only support "refresh" in documentation [1]. >> > However, we can also supply the "copy_data" option, and the code is: >> > >> >> I think there is a reference to the 'copy_data' option as well. There >> is a sentence saying: "Additionally, refresh options as described >> under REFRESH PUBLICATION may be specified." and then if you Refresh >> option, there we do mention about 'copy_data', isn't that sufficient? > > Right. It looks like the copy_option is indirectly mentioned with the > statement "Additionally, refresh options as described under REFRESH > PUBLICATION may be specified." under "set_publication_option". IMHO, > we can keep it that way. > My bad. It may be sufficient. Sorry for noise. -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd.