Re: pg_rewind with --write-recovery-conf option doesn't write dbname to primary_conninfo value.

On Sun, Feb 2, 2025 at 9:50 PM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Dear Sawada-san,
>
> > I think it's a good idea to support it at least on HEAD. I've attached
> > a patch for that.
>
> +1. I've confirmed that pg_rewind and -R can't output dbname for now,
> and your patch allows to do it.
>
> Few comments for your patch.

Thank you for reviewing the patch!

>
> 1.
>
> pg_basebackup.sgml has below description. I feel this can be ported to
> pg_rewind.sgml as well.
>
> ```
> The dbname will be recorded only if the dbname was
> specified explicitly in the connection string or <link linkend="libpq-envars">
> environment variable</link>.
> ```

Agreed, will fix.

>
> 2.
> I'm not sure whether recovery_gen.h/c is a correct place for the exported function
> GetDbnameFromConnectionOptions(). The function itself does not handle
> postgresql.cuto.conf file.
> I seeked other header files and felt that connect_utils.h might be.
>
> ```
> /*-------------------------------------------------------------------------
> *
> * Facilities for frontend code to connect to and disconnect from databases.
> ```

But this function neither connects to nor disconnects from databases, either.

>
> Another idea is to change the third API to accept the whole connection string, and
> it extracts dbname from it. In this approach we can make GetDbnameFromConnectionOptions()
> to static function - which does not feel strange for me.

I'm concerned that it reduces the usability; users (or existing
extensions) would need to construct the whole connection string just
to pass the database name. If we want to avoid exposing
GetDbnameFromConnectionOptions(), I'd introduce another exposed
function for that, say GenerateRecoveryConfigWithConnStr().

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com