Re: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility.

From: Shubham Khanna <khannashubham1197(at)gmail(dot)com>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: Euler Taveira <euler(at)eulerto(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility.
Date: 2025-03-18 10:31:36
Message-ID: CAHv8RjJVv76c=ojjcP+p1BOLD_x2gaDSSVxsGOpJ-wUC1mx6mw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 18, 2025 at 12:07 PM David G. Johnston
<david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
>
> On Monday, March 17, 2025, Shubham Khanna <khannashubham1197(at)gmail(dot)com> wrote:
>>
>>
>> I have added validation for "all" to address both issues at once.
>>
>
> Usage needs to be changed to refer to object types and we should try and specify which are valid there too.
>

Fixed.

> The sgml docs need to mention “all” as a valid value and what it means (namely, if new object types are added and “all” is specified those new types will be removed as well). Suggest not using it in scripts.
>

As suggested by Amit in [1], I have removed the provision for "all" as
a valid option for --remove. This keeps the implementation focused on
the current supported object type (publications).

> There are still more places where “object type” is needed instead of just “object”. I’ll pin-point or fix tomorrow if you don’t beat me to them.
>

I identified and fixed a few instances where "object type" was needed
instead of just "object." Please let me know if I missed any other
occurrences.

> It would be good if we could get this to play nicely with —dry-run; maybe connecting to the source for the queries instead of the target. That would help alleviate my issue with the current auto-drop behavior.
>

In the v18 patch attached at [2], I have removed the second test that
verified publications are not removed when --remove is not specified.
Now, the test suite only verifies that pg_createsubscriber with
--remove correctly removes publications from the subscriber node. This
reduces redundancy while still validating the core functionality of
the --remove option.
IIUC, for testing with --dry-run, we can directly check the relevant
stdout logs (e.g., "dropping publication 'test_pub1' ...") to verify
the call without actually dropping the publications.
However, IMO, using --dry-run alone would miss code coverage for the
actual drop publication execution part.
Please let me know if I misunderstood your point. I will update this
accordingly once I have more clarity.

The attached patch contains the suggested changes,

[1] - https://www.postgresql.org/message-id/CAA4eK1JHD0fmyMkTe_y84gJ--WWPLVFo6kJMNxvFdcDs7nXjbQ%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/CAHv8RjK8q%2BmWPWPh9K7CeH2tKF31vGn%2BoPV3qY7pdPCmtbjzkg%40mail.gmail.com

Thanks and regards,
Shubham Khanna.

Attachment Content-Type Size
v19-0001-Support-for-dropping-all-publications-in-pg_crea.patch application/octet-stream 12.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2025-03-18 10:32:21 Re: Reducing memory consumed by RestrictInfo list translations in partitionwise join planning
Previous Message Jakub Wartak 2025-03-18 10:19:32 Re: Draft for basic NUMA observability