Re: Enhance 'pg_createsubscriber' to retrieve databases automatically when no database is provided.

On Wed, Mar 26, 2025 at 2:05 AM Euler Taveira <euler(at)eulerto(dot)com> wrote:
>
> On Tue, Mar 25, 2025, at 8:07 AM, Shubham Khanna wrote:
>
> Apologies for the oversight. I have attached the patches now. Please
> find them included here.
>
>
> I started looking at this patch. When I started reading the commit message, I
> didn't understand why the options that provide names to objects are
> incompatible with --all option. I agree that --all and --database should be
> incompatible but the others shouldn't. We already have a check saying that the
> number of specified objects cannot be different from the number of databases.
> Why don't rely on it instead of forbidding these options?
>

We must ensure to match the order of objects specified with the
database as well (The doc says: The order of the multiple publication
name switches must match the order of database switches.). It is easy
for users to do that when explicitly specifying databases with -d
switches but not with --all case. I can see a way to extend the
current functionality to allow just fetching databases from the
publisher and displaying them to the user, then we can expect the user
to specify the names of other objects in the same order but not
otherwise.

>
> What happens if you don't specify the dbname in -P option?
>
> + /* Establish a connection to the source server */
> + conn = connect_database(opt->pub_conninfo_str, true);
>
> It defaults to user name. If you are using another superuser (such as admin)
> the connection might fail if there is no database named 'admin'. vacuumdb that
> has a similar --all option, uses another option (--maintenance-db) to discover
> which databases should be vacuumed. I'm not suggesting to add the
> --maintenance-db option. I wouldn't like to hardcode a specific database
> (template1, for example) if there is no dbname in the connection string.
> Instead, suggest the user to specify dbname into -P option. An error message
> should be provided saying the exact reason: no dbname was specified.
>

But why shouldn't we use the same specification as vacuumdb, which is:
"If not specified, the postgres database will be used, or if that does
not exist, template1 will be used."?

>
> I checked the applications that provide multiple synopsis using the following command.
>
> grep '<cmdsynopsis' doc/src/sgml/ref/*.sgml | awk '{print $1}' | sort | uniq -c
>
> There are just 3 applications that have multiple cmdsynopsis. pgbench has 2
> distinct tasks (load and run) so it makes sense to have 2 synopsis. pg_ctl has
> multiple tasks and also deserves multiple synopsis. The complexity introduced
> into vacuumdb (per table, per schema) seems to justify multiple synopsis too.
> However, the same logic doesn't apply here. IMO 0002 shouldn't be applied
> because the additional option (--all) doesn't justify multiple synopsis for
> syntax clarification.
>

Yeah, also, vacuumdb doesn't have a separate line for --all in
synopsis. So, I agree with you about not adding '--all' option in the
synopsis.

--
With Regards,
Amit Kapila.