On Tue, Mar 18, 2025 at 12:07 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:
>
> On Monday, March 17, 2025, Shubham Khanna <khannashubham1197@gmail.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
ifyou 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
insteadof 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.
