From 34258f0f6ad2410bfec9d1a398d121ce466f79a2 Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Mon, 3 Jul 2023 13:50:31 +0200 Subject: [PATCH v3] Allow specifying a dbname in pg_basebackup connection string Normally it doesn't really matter which dbname is used in the connection string that pg_basebackup and other physical replication CLI tools use. The reason being, that physical replication does not work at the database level, but instead at the server level. So you will always get the data for all databases. However, when there's a proxy, such as PgBouncer, in between the client and the server, then might very well matter. Because this proxy might want to route the connection to a different server depending on the dbname parameter in the startup packet. This patch changes the creation of the connection string key value pairs, so that the following command will actually include dbname=postgres in the startup packet to the server: ``` pg_basebackup --dbname 'dbname=postgres port=6432' ``` This also applies to other physical replication CLI tools like pg_receivewal. --- doc/src/sgml/ref/pg_basebackup.sgml | 11 +++++++++-- doc/src/sgml/ref/pg_receivewal.sgml | 11 +++++++++-- src/bin/pg_basebackup/streamutil.c | 29 ++++++++++++++++++----------- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index 79d3e657c32..88d20abe723 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -752,8 +752,15 @@ PostgreSQL documentation The option is called --dbname for consistency with other client applications, but because pg_basebackup - doesn't connect to any particular database in the cluster, any database - name in the connection string will be ignored. + doesn't connect to any particular database in the cluster, supplying a + specific database name in the connection string won't cause + pg_basebackup to behave any + differently when connecting directly to a + PostgreSQL server. + However, if you are connecting to PostgreSQL + through a proxy, then it's possible that this proxy does use the + supplied databasename to make certain decisions, such as to which + cluster to route the connection. diff --git a/doc/src/sgml/ref/pg_receivewal.sgml b/doc/src/sgml/ref/pg_receivewal.sgml index cecc7daec97..5e3ff238159 100644 --- a/doc/src/sgml/ref/pg_receivewal.sgml +++ b/doc/src/sgml/ref/pg_receivewal.sgml @@ -316,8 +316,15 @@ PostgreSQL documentation The option is called --dbname for consistency with other client applications, but because pg_receivewal - doesn't connect to any particular database in the cluster, database - name in the connection string will be ignored. + doesn't connect to any particular database in the cluster, supplying a + specific database name in the connection string won't cause + pg_receivewal to behave any + differently when connecting directly to a + PostgreSQL server. + However, if you are connecting to PostgreSQL + through a proxy, then it's possible that this proxy does use the + supplied database name to make certain decisions, such as to which + cluster to route the connection. diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c index 75ab9e56f3e..dbd08ab1722 100644 --- a/src/bin/pg_basebackup/streamutil.c +++ b/src/bin/pg_basebackup/streamutil.c @@ -79,9 +79,6 @@ GetConnection(void) /* * Merge the connection info inputs given in form of connection string, * options and default values (dbname=replication, replication=true, etc.) - * Explicitly discard any dbname value in the connection string; - * otherwise, PQconnectdbParams() would interpret that value as being - * itself a connection string. */ i = 0; if (connection_string) @@ -92,18 +89,24 @@ GetConnection(void) for (conn_opt = conn_opts; conn_opt->keyword != NULL; conn_opt++) { - if (conn_opt->val != NULL && conn_opt->val[0] != '\0' && - strcmp(conn_opt->keyword, "dbname") != 0) + if (conn_opt->val != NULL && conn_opt->val[0] != '\0') argcount++; } 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' && - strcmp(conn_opt->keyword, "dbname") != 0) + if (conn_opt->val != NULL && conn_opt->val[0] != '\0') { keywords[i] = conn_opt->keyword; values[i] = conn_opt->val; @@ -115,11 +118,11 @@ GetConnection(void) { keywords = pg_malloc0((argcount + 1) * sizeof(*keywords)); values = pg_malloc0((argcount + 1) * sizeof(*values)); + keywords[i] = "dbname"; + values[i] = dbname; + i++; } - keywords[i] = "dbname"; - values[i] = dbname == NULL ? "replication" : dbname; - i++; keywords[i] = "replication"; values[i] = dbname == NULL ? "true" : "database"; i++; @@ -171,7 +174,11 @@ GetConnection(void) values[i] = NULL; } - tmpconn = PQconnectdbParams(keywords, values, true); + /* + * Only expand dbname when we did not already parse the argument as a + * connection string ourselves. + */ + tmpconn = PQconnectdbParams(keywords, values, !connection_string); /* * If there is too little memory even to allocate the PGconn object base-commit: 63956bed7b10fb3bb5fe3f74250a33c9e226a921 -- 2.34.1