From: | vignesh C <vignesh21(at)gmail(dot)com> |
---|---|
To: | Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> |
Cc: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Japin Li <japinli(at)hotmail(dot)com>, Euler Taveira <euler(at)eulerto(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Identify missing publications from publisher while create/alter subscription. |
Date: | 2022-02-13 14:04:05 |
Message-ID: | CALDaNm197QxEvfQ9Cip63rP4RQeO3HEiy=26TtnCEF4_MTQvSA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Feb 11, 2022 at 7:14 PM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>
> I have spent little time looking at the latest patch. The patch looks
> to be in good shape as it has already been reviewed by many people
> here, although I did get some comments. Please take a look and let me
> know your thoughts.
>
>
> + /* Try to connect to the publisher. */
> + wrconn = walrcv_connect(sub->conninfo, true, sub->name, &err);
> + if (!wrconn)
> + ereport(ERROR,
> + (errmsg("could not connect to the publisher: %s", err)));
>
> I think it would be good to also include the errcode
> (ERRCODE_CONNECTION_FAILURE) here?
Modified
> --
>
> @@ -514,6 +671,8 @@ CreateSubscription(ParseState *pstate,
> CreateSubscriptionStmt *stmt,
>
> PG_TRY();
> {
> + check_publications(wrconn, publications, opts.validate_publication);
> +
>
>
> Instead of passing the opts.validate_publication argument to
> check_publication function, why can't we first check if this option is
> set or not and accordingly call check_publication function? For other
> options I see that it has been done in the similar way for e.g. check
> for opts.connect or opts.refresh or opts.enabled etc.
Modified
> --
>
> Above comment also applies for:
>
> @@ -968,6 +1130,8 @@ AlterSubscription(ParseState *pstate,
> AlterSubscriptionStmt *stmt,
> replaces[Anum_pg_subscription_subpublications - 1] = true;
>
> update_tuple = true;
> + connect_and_check_pubs(sub, stmt->publication,
> + opts.validate_publication);
>
Modified
> --
>
> + <para>
> + When true, the command verifies if all the specified publications
> + that are being subscribed to are present in the publisher and throws
> + an error if any of the publication doesn't exist. The default is
> + <literal>false</literal>.
>
> publication -> publications (in the 4th line : throw an error if any
> of the publication doesn't exist)
>
> This applies for both CREATE and ALTER subscription commands.
Modified
Thanks for the comments, the attached v14 patch has the changes for the same.
Regard,s
Vignesh
Attachment | Content-Type | Size |
---|---|---|
v14-0001-Identify-missing-publications-from-publisher-whi.patch | text/x-patch | 19.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Dmitry Dolgov | 2022-02-13 14:33:13 | Re: Avoiding smgrimmedsync() during nbtree index builds |
Previous Message | vignesh C | 2022-02-13 14:02:30 | Re: Identify missing publications from publisher while create/alter subscription. |