On Thu, Sep 25, 2025 at 1:15 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
> On Sep 25, 2025, at 15:07, Shubham Khanna <khannashubham1197@gmail.com> wrote:
>
>
>
> 1.
> ```
> + if (dbinfo->made_publication)
> + drop_publication(conn, dbinfo->pubname, dbinfo->dbname,
> + &dbinfo->made_publication);
> + else
> + pg_log_info("preserve existing publication \"%s\" in database \"%s\"",
> + dbinfo->pubname, dbinfo->dbname);
> + }
> ```
>
> Should we preserve “|| dry_run”? Because based on the old comment, in dry-run mode, even if we don’t create
publications,we still want to inform the user.
>
>
> We don’t need to add an explicit "|| dry_run" here, since the
> made_publication flag already accounts for that case. In dry-run mode,
> no publications are actually created, so made_publication is never
> set. This ensures we still hit the “preserve existing publication …”
> branch and inform the user accordingly.
>
>
> I doubt that. Looking the code in create_publication():
> if (!dry_run)
> {
> res = PQexec(conn, str->data);
> if (PQresultStatus(res) != PGRES_COMMAND_OK)
> {
> pg_log_error("could not create publication \"%s\" in database \"%s\": %s",
> dbinfo->pubname, dbinfo->dbname, PQresultErrorMessage(res));
> disconnect_database(conn, true);
> }
> PQclear(res);
> }
>
> /* For cleanup purposes */
> dbinfo->made_publication = true;
>
> made_publication will always be set regardless of dry_run.
>
You’re right — I made a mistake in my earlier explanation.
made_publication is always set in create_publication(), regardless of
dry-run. I have double-checked the dry-run output across the cases,
and from what I can see the messages are being logged correctly.
If you think there’s a specific combination where the dry-run logging
isn’t behaving as expected, could you point me to it? From my testing
it looks fine, but I want to be sure I’m not missing a corner case.
Thanks and regards,
Shubham Khanna.
