From: | Shubham Khanna <khannashubham1197(at)gmail(dot)com> |
---|---|
To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
Cc: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Add support for specifying tables in pg_createsubscriber. |
Date: | 2025-09-15 05:24:19 |
Message-ID: | CAHv8RjLLCJrNrz6cO5eBGTpqQ=H7EQMRqcfU7S6kU7SiiP4Dnw@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Sep 15, 2025 at 6:01 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Shubham,
>
> IIUC the v6 will be rewritten to remove the new option, in favour of
> just redefining the --publication option behaviour.
>
> So, much of the current v6 will become obsolete. The below comment is
> just for one piece of code that I thought will survive the rewrite.
>
> ======
> src/bin/pg_basebackup/pg_createsubscriber.c
>
> setup_publisher:
>
> 1.
> + /*
> + * Check if publication already exists when
> + * --reuse-existing-publications is specified
> + */
> + if (opt->reuse_existing_pubs && check_publication_exists(conn,
> dbinfo[i].pubname, dbinfo[i].dbname))
> + {
> + pg_log_info("using existing publication \"%s\" in database \"%s\"",
> + dbinfo[i].pubname, dbinfo[i].dbname);
> + make_pub = false;
> + }
> +
> /*
> * Create publication on publisher. This step should be executed
> * *before* promoting the subscriber to avoid any transactions between
> * consistent LSN and the new publication rows (such transactions
> * wouldn't see the new publication rows resulting in an error).
> */
> - create_publication(conn, &dbinfo[i]);
> + if (make_pub)
> + {
> + create_publication(conn, &dbinfo[i]);
> + dbinfo[i].made_publication = true;
> + if (opt->reuse_existing_pubs)
> + pg_log_info("created publication \"%s\" in database \"%s\"",
> + dbinfo[i].pubname, dbinfo[i].dbname);
> + }
> + else
> + dbinfo[i].made_publication = false;
>
> I think there are still too many if/else here. This logic can be
> simplified like below:
>
> if (check_publication_exists(...))
> {
> pg_log_info("using existing publication...");
> dbinfo[i].made_publication = false;
> }
> else
> {
> create_publication(conn, &dbinfo[i]);
> pg_log_info("created publication ...");
> dbinfo[i].made_publication = true;
> }
>
I have now removed the extra option and reworked the patch so that the
behavior is handled directly through the --publication option. This
way, the command will either reuse an existing publication (if it
already exists) or create a new one, making it simpler and more
intuitive for users.
The attached patch contains the suggested changes.
Thanks and regards,
Shubham Khanna.
Attachment | Content-Type | Size |
---|---|---|
v7-0001-Support-existing-publications-in-pg_createsubscri.patch | application/octet-stream | 9.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Chao Li | 2025-09-15 05:31:59 | Re: Fix missing EvalPlanQual recheck for TID scans |
Previous Message | shveta malik | 2025-09-15 04:55:07 | Re: Conflict detection for update_deleted in logical replication |