public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Have pg_basebackup write "dbname" in "primary_conninfo"?
Date: Wed, 21 Feb 2024 12:04:24 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+TgmoaLRCcCeY3_n7h0rw-r-UfJt_GiFZiJQ1pFZ-iQNUozug@mail.gmail.com>
References: <CA+TgmoYfagyp5AvKMfGpRUgXNJZ0ymuqiWTzOc2tV_ZMM0UkCA@mail.gmail.com>
<TYCPR01MB1207777150173002564A753B8F5502@TYCPR01MB12077.jpnprd01.prod.outlook.com>
<CA+TgmoaLRCcCeY3_n7h0rw-r-UfJt_GiFZiJQ1pFZ-iQNUozug@mail.gmail.com>
At Tue, 20 Feb 2024 19:56:10 +0530, Robert Haas <[email protected]> wrote in
> It seems like maybe somebody should look into why this is happening,
> and perhaps fix it.
GetConnection()@streamutil.c wants to ensure conninfo has a fallback
database name ("replication"). However, the function seems to be
ignoring the case where neither dbname nor connection string is given,
which is the first case Kuroda-san raised. The second case is the
intended behavior of the function.
> /* pg_recvlogical uses dbname only; others use connection_string only. */
> Assert(dbname == NULL || connection_string == NULL);
And the function incorrectly assumes that the connection string
requires "dbname=replication".
> * Merge the connection info inputs given in form of connection string,
> * options and default values (dbname=replication, replication=true, etc.)
But the name is a pseudo database name only used by pg_hba.conf
(maybe) , which cannot be used as an actual database name (without
quotation marks, or unless it is actually created). The function
should not add the fallback database name because the connection
string for physical replication connection doesn't require the dbname
parameter. (attached patch)
About the proposed patch, pg_basebackup cannot verify the validity of
the dbname. It could be problematic.
Although I haven't looked the original thread, it seems that the
dbname is used only by pg_sync_replication_slots(). If it is true,
couldn't we make the SQL function require a database name to make a
connection, instead of requiring it in physical-replication conninfo?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 56d1b15951..da63a04de4 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -78,7 +78,7 @@ GetConnection(void)
/*
* Merge the connection info inputs given in form of connection string,
- * options and default values (dbname=replication, replication=true, etc.)
+ * options and default values (replication=true, etc.)
*/
i = 0;
if (connection_string)
@@ -96,14 +96,6 @@ GetConnection(void)
keywords = pg_malloc0((argcount + 1) * sizeof(*keywords));
values = pg_malloc0((argcount + 1) * sizeof(*values));
- /*
- * Set dbname here already, so it can be overridden by a dbname in the
- * connection string.
- */
- keywords[i] = "dbname";
- values[i] = "replication";
- i++;
-
for (conn_opt = conn_opts; conn_opt->keyword != NULL; conn_opt++)
{
if (conn_opt->val != NULL && conn_opt->val[0] != '\0')
Attachments:
[text/plain] do_not_set_fallback_dbname.txt (969B, ../[email protected]/2-do_not_set_fallback_dbname.txt)
download | inline diff:
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 56d1b15951..da63a04de4 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -78,7 +78,7 @@ GetConnection(void)
/*
* Merge the connection info inputs given in form of connection string,
- * options and default values (dbname=replication, replication=true, etc.)
+ * options and default values (replication=true, etc.)
*/
i = 0;
if (connection_string)
@@ -96,14 +96,6 @@ GetConnection(void)
keywords = pg_malloc0((argcount + 1) * sizeof(*keywords));
values = pg_malloc0((argcount + 1) * sizeof(*values));
- /*
- * Set dbname here already, so it can be overridden by a dbname in the
- * connection string.
- */
- keywords[i] = "dbname";
- values[i] = "replication";
- i++;
-
for (conn_opt = conn_opts; conn_opt->keyword != NULL; conn_opt++)
{
if (conn_opt->val != NULL && conn_opt->val[0] != '\0')
view thread (13+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Have pg_basebackup write "dbname" in "primary_conninfo"?
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox