From b0fa4c256f637f747f6753738c54f93b89e1b99e Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Mon, 3 Jul 2023 13:50:31 +0200 Subject: [PATCH v1] 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. --- src/bin/pg_basebackup/streamutil.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c index 15514599c4e..38cae8ff190 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: a4cfeeca5a97f2b5969c31aa69ba775af95ee5a3 -- 2.34.1